hooks icon indicating copy to clipboard operation
hooks copied to clipboard

feat: add useTheme

Open ianzone opened this issue 1 year ago • 13 comments

[中文版模板 / Chinese template]

🤔 This is a ...

  • [x] New feature
  • [ ] Bug fix
  • [x] Site / documentation update
  • [x] Demo update
  • [ ] TypeScript definition update
  • [ ] Bundle size optimization
  • [ ] Performance optimization
  • [ ] Enhancement feature
  • [x] Internationalization
  • [ ] Refactoring
  • [ ] Code style optimization
  • [x] Test Case
  • [x] Branch merge
  • [ ] Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English add useTheme hook
🇨🇳 Chinese 增加 useTheme 钩子

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • [x] Doc is updated/provided or not needed
  • [x] Demo is updated/provided or not needed
  • [x] TypeScript definition is updated/provided or not needed
  • [x] Changelog is provided or not needed

ianzone avatar Aug 01 '24 10:08 ianzone

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

:white_check_mark: liuyib
:white_check_mark: ianzone
:x: crazylxr
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Aug 01 '24 10:08 CLAassistant

很有意思的一个 hook,麻烦提一个 RFC,我们先在 issue 里面详细讨论一下 api 的设计呢

crazylxr avatar Aug 05 '24 01:08 crazylxr

请问还有要改的吗?

ianzone avatar Aug 07 '24 03:08 ianzone

需要再许可一次

ianzone avatar Aug 08 '24 06:08 ianzone

一般做动态主题时,会给 html 加上特定类名(如 .dark)或者特定属性(data-theme="dark") 等,来实现动态换肤,所以这个 hook 应该还需要加个回调参数 onChange,这样 theme 变动时,用户可以自己完成上述这些操作。@ianzone 你觉着呢?

代码我更新了一部分,你改之前 pull 一下

liuyib avatar Aug 08 '24 06:08 liuyib

一般做动态主题时,会给 html 加上特定类名(如 .dark)或者特定属性(data-theme="dark") 等,来实现动态换肤,所以这个 hook 应该还需要加个回调参数 onChange,这样 theme 变动时,用户可以自己完成上述这些操作。@ianzone 你觉着呢?

代码我更新了一部分,你改之前 pull 一下

改好了

ianzone avatar Aug 08 '24 08:08 ianzone

还有哪里需要改的吗?

ianzone avatar Aug 12 '24 01:08 ianzone

还望及时处理,不然一直落后于主分支。

ianzone avatar Aug 12 '24 05:08 ianzone

落后于主分支不用关注,merge的那一下会有人同步主分支的

------------------ Original ------------------ From: Yiheng @.> Date: Mon,Aug 12,2024 1:48 PM To: alibaba/hooks @.> Cc: 云泥 @.>, Comment @.> Subject: Re: [alibaba/hooks] feat: add useTheme (PR #2617)

还望及时处理,不然一直落后于主分支。

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

liuyib avatar Aug 12 '24 09:08 liuyib

一般做动态主题时,会给 html 加上特定类名(如 .dark)或者特定属性(data-theme="dark") 等,来实现动态换肤,所以这个 hook 应该还需要加个回调参数 onChange,这样 theme 变动时,用户可以自己完成上述这些操作。@ianzone 你觉着呢?

代码我更新了一部分,你改之前 pull 一下

这个 onChange 是不是也可以不加呢,直接监听 theme 的变化就好了?

crazylxr avatar Aug 21 '24 13:08 crazylxr

另外这个 hook 叫 useTheme 是不是不太合适呢,看到这个 hook 的名称无法很明确的知道这个 hook 干什么的,这个跟 darkMode 比较相关,是不是可以考虑 hook 的名字结合一下这个

crazylxr avatar Aug 21 '24 14:08 crazylxr

一般做动态主题时,会给 html 加上特定类名(如 .dark)或者特定属性(data-theme="dark") 等,来实现动态换肤,所以这个 hook 应该还需要加个回调参数 onChange,这样 theme 变动时,用户可以自己完成上述这些操作。@ianzone 你觉着呢? 代码我更新了一部分,你改之前 pull 一下

这个 onChange 是不是也可以不加呢,直接监听 theme 的变化就好了?

赞同

ianzone avatar Aug 22 '24 01:08 ianzone

另外这个 hook 叫 useTheme 是不是不太合适呢,看到这个 hook 的名称无法很明确的知道这个 hook 干什么的,这个跟 darkMode 比较相关,是不是可以考虑 hook 的名字结合一下这个

但是这个hooks包括了light, dark, system 三种mode,而且确实和主题相关,所以我认为命名合适

ianzone avatar Aug 22 '24 01:08 ianzone

没问题的话可否合并一下?我需要用这个钩子。

ianzone avatar Sep 11 '24 06:09 ianzone

这是一个新加的 hooks 却没有使用 minor 的版本号更新,

会对一部分人造成影响,实测中由于 v3.8.2 新加入的 useTheme() 使用了 window 对象,以往能在 nodejs 环境里跑通的单元测试都失败了

Airkro avatar Dec 05 '24 01:12 Airkro

这是一个新加的 hooks 却没有使用 minor 的版本号更新,会对一部分人造成影响

实测中由于 v3.8.2 新加入的 useTheme() 使用了 window 对象,以往能在 nodejs 环境里跑通的单元测试都失败了

对,很遗憾,昨天晚上再发布完写 release log 的时候才发现这个问题。

crazylxr avatar Dec 05 '24 02:12 crazylxr

https://github.com/alibaba/hooks/issues/2689 看来已经发现问题了,现在会导致依赖 ahooks 的仓库都会出现这个问题

Przeblysk avatar Dec 05 '24 05:12 Przeblysk

这是一个新加的 hooks 却没有使用 minor 的版本号更新,

会对一部分人造成影响,实测中由于 v3.8.2 新加入的 useTheme() 使用了 window 对象,以往能在 nodejs 环境里跑通的单元测试都失败了

3.8.4 支持了 ssr ,解决了该问题

crazylxr avatar Dec 05 '24 07:12 crazylxr