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: add getThemeVar file to support theme config and fix 4.1.2 them… #23171

Merged
merged 2 commits into from Apr 12, 2020

Conversation

AshoneA
Copy link
Contributor

@AshoneA AshoneA commented Apr 12, 2020

…e config breaking change

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / document update
  • Component style update
  • TypeScript definition update
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

Firstly, I have to say sorry for making a breaking change in 4.1.2 #22934 . It make issues like #23163 and #23154 . So, for not to breaking old config, dark-theme and compact-theme still like before 4.1.2.
Now, for more backward compatible and simple for config mode. Use const { getThemeVar } = require('antd/dist/getThemeVar'); to config theme.

📝 Changelog

Language Changelog
🇺🇸 English fix Variable is undefined when using dark and compact theme. Support getThemeVariables for an easy way to get theme variables.
🇨🇳 Chinese 修复引用暗黑或紧凑主题时提示 Variable is undefined 的问题。 提供 getThemeVariables 方便获取对应主题变量。

☑️ Self Check before Merge

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

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

View rendered docs/react/customize-theme.en-US.md
View rendered docs/react/customize-theme.zh-CN.md

Copy link

@tests-checker tests-checker bot left a comment

Could you please add tests to make sure this change works as expected?

@ant-design-bot
Copy link
Contributor

@ant-design-bot ant-design-bot commented Apr 12, 2020

@codesandbox-ci
Copy link

@codesandbox-ci codesandbox-ci bot commented Apr 12, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 47c1f12:

Sandbox Source
antd reproduction template Configuration

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Apr 12, 2020

因为之前是通过解构对象的方式做配置,所以在不动用户配置的情况下,只能还是维持 dark-theme.js 的内容。为了以后这种情况再次发生,使用自己内部封装的 getThemeVar 函数让用户去配置,以后可更加灵活的在不动用户配置的情况下修复问题。

@codecov
Copy link

@codecov codecov bot commented Apr 12, 2020

Codecov Report

Merging #23171 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #23171   +/-   ##
=======================================
  Coverage   98.35%   98.35%           
=======================================
  Files         365      365           
  Lines        7285     7285           
  Branches     1999     1951   -48     
=======================================
  Hits         7165     7165           
  Misses        120      120           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c2473a...47c1f12. Read the comment docs.

@afc163
Copy link
Member

@afc163 afc163 commented Apr 12, 2020

提供 getThemeVar 配置主题和修复 4.1.2 主题配置不兼容

修复了用户遇到的什么问题?

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Apr 12, 2020

修复了用户遇到的什么问题?

因为这个 #22934 其实是个 breaking change,#23154#23163 是由此产生的,所以之前的配置都得加个 ...defaultTheme,感觉很不好。

@afc163
Copy link
Member

@afc163 afc163 commented Apr 12, 2020

getThemeVar 看上去也不是一个能向前兼容的配置。

const defaultTheme = require('antd/dist/default-theme');
const darkTheme = require('antd/dist/dark-theme');
const compactTheme = require('antd/dist/compact-theme');
const { getThemeVar } = require('antd/dist/getThemeVar');
Copy link
Member

@afc163 afc163 Apr 12, 2020

Choose a reason for hiding this comment

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

Suggested change
const { getThemeVar } = require('antd/dist/getThemeVar');
const { getThemeVariables } = require('antd/dist/themes');

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Apr 12, 2020

但是之前 dark-theme 还是跟以前保持一致,所以以前的配置也是可以用的。
加了 getThemeVar 作为另一种配置方式。

@afc163
Copy link
Member

@afc163 afc163 commented Apr 12, 2020

所以这个 PR 也能回滚到 4.1.1 的主题用法了是么,直接引用 darkTheme 不用配置 defaultTheme 也能跑。

@afc163
Copy link
Member

@afc163 afc163 commented Apr 12, 2020

修复引用暗黑或紧凑主题时提示 Variable is undefined 的问题。

提供 getThemeVariables 方便获取对应主题变量。

这么写 changelog 好了。

@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Apr 12, 2020

所以这个 PR 也能回滚到 4.1.1 的主题用法了是么,直接引用 darkTheme 不用配置 defaultTheme 也能跑。

对的,但是混合模式的话就得用 getThemeVar 来开启了。之前方式配置的混合模式不行了,不过混合 4.1.2 才能开启,应该还没多少人用,感觉问题不大。

@afc163
Copy link
Member

@afc163 afc163 commented Apr 12, 2020

我明天发一个版本尽量减小影响。

@afc163 afc163 merged commit 65293f6 into ant-design:master Apr 12, 2020
21 checks passed
@AshoneA
Copy link
Contributor Author

@AshoneA AshoneA commented Apr 13, 2020

我再检查一下。

xrkffgg pushed a commit that referenced this issue Apr 13, 2020
#23171)

* feat: add getThemeVar file to support theme config and fix 4.1.2 theme config breaking change

* patch
@afc163 afc163 mentioned this pull request Apr 13, 2020
xrkffgg pushed a commit that referenced this issue Apr 16, 2020
#23171)

* feat: add getThemeVar file to support theme config and fix 4.1.2 theme config breaking change

* patch
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.

None yet

3 participants