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

fix: add grayscale colors (neutral palette) to colors.less #31129

Closed
wants to merge 6 commits into from

Conversation

stevula
Copy link

@stevula stevula commented Jun 25, 2021

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?) - this adds Less variables for the grayscale colors mentioned in the docs. They were previously undefined.

🔗 Related issue link

💡 Background and solution

  1. The antd docs on color palettes mention a gray palette (source):
    image

  2. ant-design-colors also includes a gray palette for use within JS (source)

  3. The @gray-* variables are undefined in the colors.less (bundled with antd) which causes a compiler error if you try to import them:
    image

Solution: added variable definitions for gray colors in colors.less

📝 Changelog

Language Changelog
🇺🇸 English Non-breaking change: applications can now import @gray-1 through @gray-10 from the Less theme.
🇨🇳 Chinese 在样式表中定义 @gray-1 ... @gray-10

☑️ Self Check before Merge

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

  • Doc is updated/provided or not needed (not needed, docs already mention gray)
  • Demo is updated/provided or not needed (not needed)
  • TypeScript definition is updated/provided or not needed (no TS changes)
  • Changelog is provided or not needed

@github-actions
Copy link
Contributor

Prepare preview

@stevula stevula marked this pull request as ready for review June 25, 2021 00:24
@codecov
Copy link

codecov bot commented Jul 15, 2021

Codecov Report

Merging #31129 (a96ee04) into master (399f073) will not change coverage.
The diff coverage is n/a.

❗ Current head a96ee04 differs from pull request most recent head bbd18fc. Consider uploading reports for the commit bbd18fc to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master    #31129   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          397       397           
  Lines         7561      7570    +9     
  Branches      2074      2134   +60     
=========================================
+ Hits          7561      7570    +9     
Impacted Files Coverage Δ
components/form/FormItem.tsx 100.00% <0.00%> (ø)
components/dropdown/dropdown-button.tsx 100.00% <0.00%> (ø)
components/table/hooks/useSelection.tsx 100.00% <0.00%> (ø)
components/input/ClearableLabeledInput.tsx 100.00% <0.00%> (ø)
components/table/hooks/useFilter/index.tsx 100.00% <0.00%> (ø)

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 399f073...bbd18fc. Read the comment docs.

@afc163
Copy link
Member

afc163 commented Jul 19, 2021

ant-design/ant-design-colors#3 (comment)

@stevula
Copy link
Author

stevula commented Jul 19, 2021

Updated to use custom neutral/grayscale palette in bbd18fc:

image
https://ant.design/docs/spec/colors#Neutral-Color-Palette

@stevula
Copy link
Author

stevula commented Aug 16, 2021

I have made the changes. Do you need anything else or can you please review again?

@stevula stevula changed the title fix: add grayscale colors to colors.less fix: add grayscale colors / neutral palette to colors.less Aug 20, 2021
@stevula stevula changed the title fix: add grayscale colors / neutral palette to colors.less fix: add grayscale colors (neutral palette) to colors.less Aug 20, 2021
@stevula
Copy link
Author

stevula commented Aug 27, 2021

@afc163 @PeachScript can you review again please?

@vagusX vagusX requested a review from afc163 September 13, 2021 02:57
@afc163
Copy link
Member

afc163 commented Sep 16, 2021

@zombieJ 后续统一看看,这是个遗留命名问题,感觉可以下一个大版本统一解决。

@rvinzent
Copy link

@afc163 @zombieJ with the next major version (v5.x) now released, is there any reason these changes didn't make it in? what can we do to ensure this feature gap is addressed?

@li-jia-nan
Copy link
Member

Conflicting files, I'm sorry to close, can be re initiated if necessary.

@li-jia-nan li-jia-nan closed this Jan 9, 2023
@rvinzent
Copy link

@li-jia-nan I don't think it's worth putting in the effort to re-open this PR if the maintainers won't commit to merging it, especially since the conflicts in this case would have been trivial to resolve.

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.

8 participants