-
Notifications
You must be signed in to change notification settings - Fork 10
modified lots of ui design #35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
demo is available on https://xk.r-ay.cn/ |
Instead of modifying the theme, it might be more popular to add a switch theme function (personal opinion, because I still like the original style🤪) |
Thanks for your work. Since I am no longer taking courses here, I am OK with any theme changes as long as they improve user experience. |
when the width is not enough, its position was unexpected.
@Errant404 ok now I have added the switch theme function :D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job ! 😍
due to the previous line-height change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CopyCourseId button should be useful. Theme switching is also a good job! Maybe we can also support multiple semesters in the future as #34 proposed. But that may require much refactoring or even a new version.
@chinggg i'm afraid to merge now because i don't know whether ci failed will influence build or deployment by github action after merging, does this repository be deployed by github action automatically or not? (ci failed may due to using yarn 2 or some dependencies updated) |
Adding 'engines ignore' due to ci failing and can't be fixed (vuejs/vue-cli#7116) seems ok now. The deployment should be completed by anyone who has access. |
The website will be deployed by @ZKLlab |
@ENDsoft233 Thanks a lot for your valuable contribution to this project. I will review the code and get back to you as soon as possible. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
已 review,感谢 PR,主要有以下几点辛苦看看:
- CopyCourseId.js、UseScheduleTableTheme.js 移到 src/mixins/common 目录下;
- Storage 存储当前主题建议使用主题名称,如 classic 和 candy,同时需做好匹配不到主题的兜底;
- 一些样式上的修改建议
src/pages/index/components/ReservedClassesList/CourseClassesList.vue
Outdated
Show resolved
Hide resolved
src/pages/index/components/ReservedClassesList/CourseClassesList.vue
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scripts 里使用环境变量要用 cross-env 库支持 Windows 平台
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 Thank you, I will merge it into the main branch. The pipeline will automatically build and deploy the website to the pre-production environment at https://pre.xk.shuosc.com. Please check if it meets the expected outcome and works properly. Once confirmed, I will release the website to the production environment.
I've checked the following changes and all seems ok.
It's time to release it, excited! XD |
@ENDsoft233 Great! Thank you for confirming. I have now completed the deployment. If you encounter any issues or have further feedback, please do not hesitate to let me know. Also, you can check the deployment log of the production environment at https://github.com/shuosc/shu-scheduling-helper/deployments/activity_log?environment=production. |
0x01 on ScheduleTable
before
after
0x02 on ReservedClassesList
before
after