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

Use the right palette variables across webapp #7862

Closed
latin-panda opened this issue Oct 24, 2022 · 4 comments
Closed

Use the right palette variables across webapp #7862

latin-panda opened this issue Oct 24, 2022 · 4 comments
Assignees
Labels
Type: Technical issue Improve something that users won't notice
Milestone

Comments

@latin-panda
Copy link
Contributor

Describe the issue
There is a comment that says:

Basic colors. WARNING: Don't use these outside this file as the variable names are not useful

But it's easily ignorable, also, in many places we're using the wrong variables for colors, we should use the palette.

Describe the improvement you'd like
Make more difficult to use the wrong color variables.

Describe alternatives you've considered
LESS doesn't have private variables, a workaround is to move the ones that we can’t use but that are needed for variable.less to a file named private-vars.less, so variable.less is the only one to import it.

@latin-panda latin-panda added the Type: Technical issue Improve something that users won't notice label Oct 24, 2022
@latin-panda latin-panda self-assigned this Oct 24, 2022
@latin-panda
Copy link
Contributor Author

The variables in private-vars.less are still accessible anywhere that imports variable.less, so this idea doesn't work. However having them in 2 files might help devs to understand the right way of using them

@latin-panda
Copy link
Contributor Author

This is ready for AT here I feel very confident about the change, the app's color should still be the same as in master.

@lorerod
Copy link
Contributor

lorerod commented Nov 22, 2022

Environment
Client platform: MacOS
Browser: Chrome
App: Webapp
Versions 1: 4.0.0
Instance: Local using cht-core.yml and cht-couchdb.yml

Versions 2: Branch7862-private-color-variables
Instance: Local using Developer setup

The app's colors look the same in both versions.

latin-panda added a commit that referenced this issue Nov 29, 2022
Ticket: #7862

Extracting color variables to another file, the ones that are not part of the palette.
The variables in `private-vars.less` are still accessible anywhere that imports `variable.less`. However having them in 2 files might help devs to understand the right way of using them.
@latin-panda
Copy link
Contributor Author

Thanks @lorerod!

This has been merged to our main branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Technical issue Improve something that users won't notice
Projects
Status: Done
Development

No branches or pull requests

2 participants