-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Refactor: Standardize border variables across components (#11602) #11614
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: Standardize border variables across components (#11602) #11614
Conversation
…ers.less in index.less
…r style variables over border width variables
for more information, see https://pre-commit.ci
lokesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@G-Sham
Thanks for your work on this!
There are some issues in this PR to work through to get it ready that I will note below. The main thing I would think about is being much less ambitious. Touch fewer files and only insert the variables in instances where you have confidence you understand what the UI component is and it's context.
Issues to address:
- There are missing Less imports that need to be resolved in a few files. You can run
docker compose run --rm home npm run-script lintto see the errors. - We should mostly avoid using the primitive variables. e.g.
@border-width,@border-width-thick. If you see a CSS rule that has a border width of 1px, avoid replacing the 1px with a variable. In almost all cases we want to use a semantic variable, and ideally a complete border style variable. If you can't find a semantic variable that works, then leave the hardcoded value or consider adding a semantic value. - Make sure the semantic variables are used in their correct context. I'm seeing multiple issues here. One examples:
In static/css/components/buttonBtn.less @border-card is used to style a chip. This chip is like a tag or badge UI component, and not a card. So in this case, you could just leave the hardcoded values as is, or create a new semantic variable in borders.less:
@border-tag: @border-width solid @color-border-subtle; // tags, chips, badges
|
Hi @lokesh, thank you so much for the detailed review! Apologies for the delay in responding—I was caught up with university exams this week. I really appreciate the mentorship here, especially regarding the semantic context. I will start working on these updates now |
|
Closing. Please feel free to reopen if you do take another pass on it. |
Closes #11602
Refactor
Technical
Refactored hardcoded border values (e.g.,
1px solid @lighter-grey,2px solid) to use the semantic design tokens defined inborders.less. This ensures consistency across the application and simplifies future theming.1px solid @lighter-greywith@border-cardfor cards, notes, and content wrappers.@border-inputand@border-width-control.2pxborders to@border-width-thick(or@focus-widthfor focus states).@border-table.legacy-borrowTable.less,legacy-datatables.less,legacy-wmd.less,buttonBtn.less,legacy.less, and others.