Skip to content

fix(button,table): move LESS functions out of LESS file and into variables #737

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 10, 2019
Merged

fix(button,table): move LESS functions out of LESS file and into variables #737

merged 2 commits into from
Jun 10, 2019

Conversation

lubber-de
Copy link
Member

@lubber-de lubber-de commented May 10, 2019 •
edited
Loading

Description

The button.less and table.less files had hardcoded LESS color math functions included (darken/lighten/saturate/desaturate/screen).
If somebody defines some color variables as CSS Variable strings ( like var(--mycssvar)) , any LESS Math functions did not work anymore and caused a LESS compiler error (because LESS always expects a valid color definition as argument)

To comply with the central color definition system we invented in 2.7.0 this PR now replaces all those functions by appropriate variables instead.

This way, if someone decides to change all/some color definitions to strings like mentioned above, the button code won't break anymore (as long as every color related saturate/darken/lighten/desaturate...etc. was adjusted in the *.variables files)

Closes

#736

@lubber-de lubber-de added type/bug Any issue which is a bug or PR which fixes a bug lang/css Anything involving CSS state/awaiting-reviews Pull requests which are waiting for reviews labels May 10, 2019
@lubber-de lubber-de added this to the 2.7.x milestone May 10, 2019
exoego
exoego previously approved these changes May 10, 2019
View reviewed changes
Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

LGTM

@lubber-de lubber-de mentioned this pull request May 10, 2019
Closed
Yahav pushed a commit to Yahav/fomanticUiColorSystem that referenced this pull request May 10, 2019
fixing less functions in less file: fomantic/Fomantic-UI#737
69d51a0
@Yahav Yahav mentioned this pull request May 10, 2019
Closed
@lubber-de lubber-de changed the title [Button] Move LESS Functions out of LESS file and into separat variables instead [Button,Table] Move LESS Functions out of LESS file and into separat variables instead May 10, 2019
@Yahav
Copy link

Yahav commented May 10, 2019

that solved the table, next one is Item:

{ [Error: error evaluating function lighten: color.toHSL is not a function in file /home/yahav/fomantic_themeColors/semantic/src/themes/default/views/item.variables line no. 169] message: 'error evaluating function lighten: color.toHSL is not a function in file /home/yahav/fomantic_themeColors/semantic/src/themes/default/views/item.variables line no. 169', stack: undefined, type: 'Runtime', filename: '/home/yahav/fomantic_themeColors/semantic/src/themes/default/views/item.variables', index: 3656, line: 169, column: 27, callLine: NaN, callExtract: undefined, extract: [ '@invertedContentColor: @invertedTextColor;', '@invertedContentLinkColor: lighten(saturate(@linkColor, 30), 25, relative);', '@invertedContentLinkHoverColor: @linkColor;' ], lineNumber: 169, fileName: '/home/yahav/fomantic_themeColors/semantic/src/themes/default/views/item.variables', name: 'Error', plugin: 'gulp-less', showProperties: true, showStack: false, __safety: { toString: [Function: bound ] } }

@lubber-de
Copy link
Member Author

@Yahav

that solved the table, next one is Item:

No, this time the LESS function is part of a *.variable file (as all the other variables in site.variables as well)
So in this case, as you modified the @linkColor variable in your theme, you also have to set the @invertedContentLinkColor to your own var(--) value, which you probably want anyway.
(remember to set it before changing the @linkColor)

@lubber-de lubber-de requested a review from exoego May 13, 2019 06:15
ColinFrick
ColinFrick approved these changes May 21, 2019
View reviewed changes
Copy link
Member

@ColinFrick ColinFrick left a comment

Choose a reason for hiding this comment

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

LGTM

y0hami
y0hami approved these changes Jun 10, 2019
View reviewed changes
Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

LGTM

@y0hami y0hami changed the title [Button,Table] Move LESS Functions out of LESS file and into separat variables instead fix(button,table): move LESS functions out of LESS file and into variables Jun 10, 2019
@y0hami y0hami merged commit e3d7eb6 into fomantic:develop Jun 10, 2019
@y0hami y0hami removed state/awaiting-reviews Pull requests which are waiting for reviews labels Jun 10, 2019
@lubber-de lubber-de modified the milestones: 2.7.x, 2.7.6 Jun 10, 2019
@lubber-de lubber-de deleted the fix/736/move_less_functions_out_of_element branch June 10, 2019 12:35
@lubber-de lubber-de mentioned this pull request Jul 26, 2023
Merged
lubber-de added a commit that referenced this pull request Jul 31, 2023
@lubber-de
fix(theme): check real colors to support css variables
4f21527 
This change allows to use CSS variables in custom themes for LESS variables which are basically used in color functions.
Although the themes changed variables are imported after the default theme, the LESS logic still tried to apply a css variable to a color function like "darken()" which of course does not work. It makes #737 complete now.

So this simple order results into "error evaluating darken()", although the variables are immediatly overridden:

@linkColor           : #4183C4;
@linkHoverColor      : darken(saturate(@linkColor, 20), 15, relative);

@linkColor: var(--some-link-color);
@linkHoverColor: var(--some-link-hover-color);
.foo {
  bar: @linkHoverColor;
}
By changing every existing color function call inside the default theme to check for a valid color value it is now possible to completely use CSS variables for a custom theme

@linkColor           : #4183C4;
@linkHoverColor      : if(iscolor(@linkColor), darken(saturate(@linkColor, 20), 15, relative), @linkColor);

@linkColor: var(--some-link-color);
@linkHoverColor: var(--some-link-hover-color);
.foo {
  bar: @linkHoverColor;
}
Succesfully results into

.foo {
  bar: var(--some-link-hover-color);
}
Simply check the above code via lesscss.org/less-preview

The minium LESS Version to support this is 3.12.0, so i had to raise the dependency 3.x version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exoego exoego exoego approved these changes

@ColinFrick ColinFrick ColinFrick approved these changes

@y0hami y0hami y0hami approved these changes

@prudho prudho Awaiting requested review from prudho

Assignees
No one assigned
Labels
lang/css Anything involving CSS type/bug Any issue which is a bug or PR which fixes a bug
Projects
None yet
Milestone
2.7.6
Development

Successfully merging this pull request may close these issues.
5 participants
@lubber-de @Yahav @exoego @ColinFrick @y0hami