Bug 34392 - Run automated Stylelint fixes on staff CSS
Summary: Run automated Stylelint fixes on staff CSS
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Owen Leonard
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-25 18:27 UTC by Owen Leonard
Modified: 2024-07-04 20:42 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Circulation function:


Attachments
Bug 34392: Run automated Stylelint fixes on staff CSS (50.29 KB, patch)
2023-07-26 14:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (50.33 KB, patch)
2023-08-03 16:01 UTC, Sam Lau
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (50.09 KB, patch)
2023-08-22 15:03 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (43.34 KB, patch)
2023-09-19 13:39 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (43.41 KB, patch)
2023-09-21 07:12 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (50.80 KB, patch)
2023-10-17 14:58 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34392: Run automated Stylelint fixes on staff CSS (53.44 KB, patch)
2023-10-18 18:52 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2023-07-25 18:27:21 UTC
As we did with Bug 31529, I think we should run automated Stylelint corrections on staff-global.scss now that the file has had significant changes in the staff interface upgrade.

Automated fixes will correct style issues like:

- lower case color hex codes
- properties out of alphabetical order
- missing empty lines or extra empty lines
- missing spaces after ":"

...according to our Stylelint configuration, .stylelintrc.json
Comment 1 Owen Leonard 2023-07-26 14:37:45 UTC Comment hidden (obsolete)
Comment 2 Sam Lau 2023-08-03 16:01:17 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2023-08-22 15:03:58 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize (ashimema) 2023-09-18 12:09:27 UTC
I feel like we should add this to the QA tools to prevent future regressions too.
Comment 5 Owen Leonard 2023-09-19 13:39:19 UTC Comment hidden (obsolete)
Comment 6 Martin Renvoize (ashimema) 2023-09-21 07:12:21 UTC
Created attachment 155967 [details] [review]
Bug 34392: Run automated Stylelint fixes on staff CSS

This patch corrects errors raised by the new stylelint configuration in
staff-global.scss. Many of the fixes were made automatically by using
the "--fix" option:

npx stylelint koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
--fix

Some examples of errors found:

    - Expected single space after ":"
    - Expected single space after "("
    - Expected single space before ")"
    - Expected empty line before comment
    - Expected no more than 1 empty line
    - Expected a trailing semicolon

Manual fixes include replacing named colors with hex codes, combining
duplicate selectors, removing empty blocks, etc.

Some warnings remain: "Unexpected !important" requires more than a
quick fix, if we should choose to address them.

To test, apply the patch and rebuild the staff interface CSS:
https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client

The changes affect CSS all around the staff interface, but there should
be no visible results of the patch. Everything should look correct.

The stylelint check should return only warnings about "!important"

npx stylelint koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize (ashimema) 2023-09-21 07:12:44 UTC
Thanks for the rebase.. Passing QA
Comment 8 Martin Renvoize (ashimema) 2023-09-21 07:20:38 UTC
Regards QA tools.. I'm reconsidering this at the moment.. should it be part of QA.. or should we simply automate it away as part of our CI process?

https://gitlab.com/koha-community/qa-test-tools/-/issues/76
Comment 9 Tomás Cohen Arazi (tcohen) 2023-10-17 13:31:50 UTC
Please rebase
Comment 10 Owen Leonard 2023-10-17 14:58:53 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2023-10-18 18:52:48 UTC
Created attachment 157342 [details] [review]
Bug 34392: Run automated Stylelint fixes on staff CSS

This patch corrects errors raised by the new stylelint configuration in
staff-global.scss. Many of the fixes were made automatically by using
the "--fix" option:

npx stylelint koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
--fix

Some examples of errors found:

    - Expected single space after ":"
    - Expected single space after "("
    - Expected single space before ")"
    - Expected empty line before comment
    - Expected no more than 1 empty line
    - Expected a trailing semicolon

Manual fixes include replacing named colors with hex codes, combining
duplicate selectors, removing empty blocks, etc.

Some warnings remain: "Unexpected !important" requires more than a
quick fix, if we should choose to address them.

To test, apply the patch and rebuild the staff interface CSS:
https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client

The changes affect CSS all around the staff interface, but there should
be no visible results of the patch. Everything should look correct.

The stylelint check should return only warnings about "!important"

npx stylelint koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss

Signed-off-by: Sam Lau <samalau@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Tomás Cohen Arazi (tcohen) 2023-10-18 19:01:05 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 13 Fridolin Somers 2023-10-25 08:06:58 UTC
Does not apply on 23.05.x
Do you think OK to not backport, regarding conflicts in future patches
Comment 14 Owen Leonard 2023-10-25 10:38:25 UTC
(In reply to Fridolin Somers from comment #13)
> Do you think OK to not backport, regarding conflicts in future patches

I think it makes sense not to backport. I'm happy to help with CSS conflicts if necessary.
Comment 15 Fridolin Somers 2023-10-28 21:09:13 UTC
Thanks for you answer.
And long live our CSS master :D