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

Misc: Font weight variables #3873

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Feb 4, 2025

Jira URL

None, misc change proposed and validated in https://forum.xwiki.org/t/css-best-practice-dont-hard-code-font-weights/14490 .

Changes

Description

  • Added three LESS variables in the global context to avoid hardcoding font weights.

Clarifications

  • Those are the most used values in the code base, 400 is the same as normal and 700 is the same as bold. From what I could see, we don't need a light font weight because it's not really used anywhere in XWiki nowadays and it'd probably be a liability for the text lisibility.

Screenshots & Video

None.

Executed Tests

None, this change has no influence on the compiled LESS and respects code style.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • 16.10.X (safe change)

* Added three LESS variables in the global context to avoid hardcoding font weights.
@michitux
Copy link
Contributor

michitux commented Feb 4, 2025

Why are you adding new LESS variables? Unless there are problems with browser support or customization support, this should be added as CSS variables imho. Also, introducing something similar to a new API doesn't seem like a misc change to me.

@michitux
Copy link
Contributor

michitux commented Feb 4, 2025

Why are you adding new LESS variables? Unless there are problems with browser support or customization support, this should be added as CSS variables imho.

While it might not seem consistent, to me the problem of adding new LESS variables is that it means that these new variables cannot be used in CSS code so for using these variables we would need to convert even more code to LESS which is absolutely against what we should be doing (converting from LESS to plain CSS). On the other hand, I think it shouldn't be any problem to use CSS variables in LESS code.

@Sereza7 Sereza7 marked this pull request as draft February 5, 2025 13:27
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.

2 participants