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

feat: use toast instead of alert #24

Merged
merged 3 commits into from
Dec 18, 2023
Merged

Conversation

Eve-Sama
Copy link
Contributor

close #HuolalaTech/page-spy-web

发现了好几个问题, 我一并修改了

问题一: 这里的拷贝和提示的文案, 不应该使用英文. 虽然copy这个单词很简单, 但是用户可能不认识, 沟通起来也麻烦, 作为用户端的文案, 还是中文比较好. 但是调试信息我没动, 因为这些是给开发看的, 保持现状就好.
解决方案: 这个直接改就行, 简单

问题二: 组件设计问题. 这个有点复杂, alert的动作目前是发生在content组件中的. alert是个业务行为, content看文件结构和命名, 像是个ui组件. 但是里面又处理了大量的业务, 比如拷贝业务. 举个例子, 按钮的点击动作, 写死为触发拷贝行为. 但并不是每次使用content都需要执行拷贝动作. 所以这里最大的问题其实是content的定位不清晰. 准确的说, 这里的content和modal应该是一个东西. 但实际上content是部分业务+部分modal的行为.
解决方案: 将content的行为精简化, 业务全部交由外部传递, 提高content的复用性.

问题三: alert的交互是无法避开的, 但是alert本身又会阻塞线程
解决方案: 我看了这个项目没有引入太多依赖, 连react都没有. 所以最适合的antd message组件都没法用. 我也理解作为sdk肯定是越小越好, 但是其他的交互体验都没有那么好, 尤其是在现有content的设计下, 并不好实现. 所以我建议引入一个纯js轻量的 toast 组件.

@Eve-Sama
Copy link
Contributor Author

新交互

Screen.Recording.2023-12-15.at.17.23.53.mov

@wqcstrong
Copy link
Collaborator

第一点可以优化下,默认英文,针对国内用户自动切换中文;
第二点接受;
第三点 SDK 不建议额外引入『非必要』的库,看了一下 toastify-js 体积 16 kb,只是为了一个弹窗功能;

@Eve-Sama
Copy link
Contributor Author

有道理, 我再改下

@Eve-Sama
Copy link
Contributor Author

试了下, 如果不是 alert, 其他的都很难实现. 或者说代码会很恶心. 那还是保持现状吧, 至少技术上没有心智负担. 只做了代码优化和拷贝文案的国际化

@wqcstrong wqcstrong merged commit 235e403 into HuolalaTech:main Dec 18, 2023
@wqcstrong
Copy link
Collaborator

感谢

@wqcstrong
Copy link
Collaborator

wqcstrong commented Dec 18, 2023

@Eve-Sama 我实现了一个简单 Toast 类,你有空帮我 review 吗

#26

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.

交互优化
2 participants