Skip to content
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

refactor: universal Modal component #666

Merged
merged 7 commits into from
May 28, 2024

Conversation

orangelckc
Copy link
Contributor

重构了设置页面的响应式布局,方便之后将部分设置项抽离至游戏页面
游戏页面的设置弹窗计划参考如下:
7371714663775_ pic

@orangelckc
Copy link
Contributor Author

游戏页面添加了设置弹窗
image
image
目前,更改了设置项后,还不能直接应用到游戏中,怀疑之前的store用法有问题,等排查后再合并

@yaolifeng0629
Copy link
Collaborator

@orangelckc 感谢你的贡献,麻烦拉取最新代码,解决下现有冲突后提交,感谢!

@orangelckc
Copy link
Contributor Author

@yaolifeng0629 已处理冲突,与当前主分支同步。

目前存在的问题:当前题目下,设置更改后,不能立即生效,但是进入下一题是会生效的。

可能需要核心团队重构当前store的使用方式后处理。

@orangelckc
Copy link
Contributor Author

另外可以优化的一个点,建议将Modal全部更新为函数式动态挂载,目前会在页面中同时存在多个modal,可能会出现渲染层级问题

@cuixiaorui
Copy link
Contributor

感谢!

  1. 这个 pr 需要拆分成2个功能
    一个是重构 dialog
    另外一个是添加 settings 弹窗

  2. 弹窗的背景色需要统一
    现在 settings 的背景色和其他的弹窗背景色不一样 看起来很突兀

  3. 原有 settings page 可以先不用改动
    后面需要重构 UI 因为现有的选择项越来越多 需要引入 tab 来分类


下一步建议:

先把这个 pr 改成只提交 重构 dialog 的功能

@fengstats
Copy link
Collaborator

fengstats commented May 28, 2024

  1. 同步主分支并解决冲突(git rebase main)✅
  • apps/client/components/DropMenu.vue(删掉了)
  • apps/client/components/main/Tool.vue(恢复远端)
  • apps/client/components/Navbar.vue(删除多余的信息,恢复远端)
  1. 强推远端代码(git push -f orangelckc HEAD:kc/setting)✅
  2. 审查并补充 CommonModal 代码(适配深色模式)✅
  3. 重构 MessageBox 组件 ✅
  4. 移除多余文件 ✅
  • composables/messageBox/modal.ts
  • components/main/MessageBox/useMessageBox.ts(函数式但只在测试文件中使用了)
  • components/main/MessageBox/tests/message-box.spec.ts
  1. 更新文件位置 ✅
  • components/main/MessageBox/MessageBox.vuecomponents/main/MessageBox.vue
  1. 更新所有使用 CommonModal 的组件样式 ✅
  2. 移除新增的 setting 弹框代码 ✅

@fengstats fengstats changed the title refactor: setting page refactor: universal Modal component May 28, 2024
@baboon-king baboon-king self-requested a review May 28, 2024 10:31
Copy link
Collaborator

@baboon-king baboon-king left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@baboon-king baboon-king merged commit c3c704e into cuixueshe:main May 28, 2024
1 check passed
@orangelckc orangelckc deleted the kc/setting branch May 28, 2024 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants