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.

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

Remove style ordering by Webpack plugin #9455

Merged
merged 2 commits into from
Sep 3, 2024
Merged

Remove style ordering by Webpack plugin #9455

merged 2 commits into from
Sep 3, 2024

Conversation

ger-benjamin
Copy link
Member

@ger-benjamin ger-benjamin commented Aug 28, 2024 •
edited
Loading

@sbrunner sbrunner changed the title Gsngeo 11 remove sstyle ordering by webpack plugin Remove style ordering by Webpack plugin Aug 28, 2024
@ger-benjamin ger-benjamin force-pushed the GSNGEO-11-remove-sstyle-ordering-by-webpack-plugin branch 7 times, most recently from 41b6a22 to 2f10739 Compare August 30, 2024 12:41
@ger-benjamin ger-benjamin requested a review from sbrunner August 30, 2024 12:42
@ger-benjamin ger-benjamin marked this pull request as ready for review August 30, 2024 12:42
@ger-benjamin ger-benjamin force-pushed the GSNGEO-11-remove-sstyle-ordering-by-webpack-plugin branch from 2f10739 to a3d2da0 Compare August 30, 2024 13:28
@ger-benjamin
Copy link
Member Author

The apps pass well now, but it looks that I've to check some examples...
I will do that next week.

@ger-benjamin ger-benjamin force-pushed the GSNGEO-11-remove-sstyle-ordering-by-webpack-plugin branch 5 times, most recently from 716d432 to d13c6bc Compare September 2, 2024 13:43
sbrunner
sbrunner reviewed Sep 2, 2024
View reviewed changes
Copy link
Member

@sbrunner sbrunner left a comment

Choose a reason for hiding this comment

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

Just some small comments, many thanks :-)

contribs/gmf/apps/desktop_alt/sass/desktop_alt.scss Outdated Show resolved Hide resolved
src/sass/components.scss Outdated Show resolved Hide resolved
src/sass/base.scss Show resolved Hide resolved
@ger-benjamin ger-benjamin force-pushed the GSNGEO-11-remove-sstyle-ordering-by-webpack-plugin branch from d13c6bc to df70168 Compare September 3, 2024 06:45
@ger-benjamin
Copy link
Member Author

What could be better with my PR is that examples (js file) generally load multi css and scss files.
That would be better to load only one scss file per example, that imports the others css and scss files (like for the app controllers).
That would be better because the css order, for two rule with the same priority, is more complicated to manage with import via js files. But I think it's okay to have examples like that (as long as the app controllers are in the best shape).

@sbrunner
Copy link
Member

sbrunner commented Sep 3, 2024

Effectively, it will be better to import only one scss file and no css file in the examples.
Don't you want to do that in another pull request?

@ger-benjamin
Copy link
Member Author

ger-benjamin commented Sep 3, 2024 •
edited
Loading

The fact that all examples have css file and not scss file made me lazy. But If you agree that would be cleaner, I can do that. But yes, in a second PR please.

@ger-benjamin ger-benjamin merged commit ff6aeae into master Sep 3, 2024
13 checks passed
@ger-benjamin ger-benjamin deleted the GSNGEO-11-remove-sstyle-ordering-by-webpack-plugin branch September 3, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbrunner sbrunner sbrunner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ger-benjamin @sbrunner