webumenia.sk
webumenia.sk copied to clipboard
SlovakNationalGallery
Feature/CSS refactoring
Description
My original intention was to introduce a dark mode for webumenia.sk. However I found out that the project CSS was due to an update and I attempted to do one.
List of changes
- add current Tailwind CSS to the project, update its build
- remove all old and unused LESS and static CSS files
- replace legacy Tailwind and Tachyons CSS with Tailwind only classes
- replace some Bootstrap classes with Tailwind
- optimize style and admin CSS builds
Todo
- use npm versions of plugins (eg. Bootstrap, CKeditor, etc.), remove its code from the repository
- introduce CSS properties
- dark mode
Type of change
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Checklist:
- [ ] I have updated the CHANGELOG
- [ ] I have requested at least 1 reviewer for this PR
- [ ] I have made corresponding changes to the documentation
Wow 🤩 . This PR looks like a miracle. We were thinking about doing all of these things before... but nobody really wanted to start. I appreciate your effort a lot. We need some time to walk through all your changes (which are huge!). And double-check any possible issues. We will definitely give you feedback soon. Thanks a lot for this. You made my day.... ✨
Thank you for the feedback. I am glad you like the direction of my efforts.
This phase is IMO done, I would like to eventually meet with you someday and talk about testing this PR and future development.
I read something about dropping / replacing the Bootstrap (in the commit messages): this is the task I would love to help you with. I also like to 'atomize' (Tailwind-ize 😄) the CSS and to work on overall refactoring.
Btw I love all your work in lab.sng. I checked the Warhol microsite in the meantime and it blew my dekel off; splendid work!
Hi @adamplesnik 👋!
Thank you very much for your effort. It'll take a while for us to review all the changes, but from skimming the code, it looks like a major step in the right direction.
Right now I'm having trouble compiling the project assets (see e.g. here). Are you able to run npm run dev locally?
Note the project uses NodeJS v14 right now. That's is not a hard requirement AFAIK but if you're using a different version, please let us know.
Hi @eronisko, thank you for the feedback.
I actually broke the build by calling Tailwind plugin in less() build. I put Tailwind into separate file, it should work now.
I am sorry about the revert, I realised too late that it would be better to leave the code formatting off for PHP files.
I am sorry about this commit mess, I reverted the initial build and then reapplied it in a hurry.
Thank you for the review @eronisko! I will take a look at issues you found and I'll push some changes.
I tried to fix all your findings. Some are fixed in CSS, but majority is in PHP/Vue by adding atomic classes.
IMO it is no longer needed to separate BS and TW styling, it is safe to enable Tailwind preflight as well.
Feedback is welcome
@adamplesnik Thanks for keeping this branch up to date and sorry for the radio silence. We're busy with preparations for digital installations right now but I'll get back to you in two weeks latest.
@eronisko Thank you for your feedback. No need to hurry, good luck with other projects ;)
@adamplesnik just so you know, I've resumed working on this pull request. I'm putting together an automated visual test suite (Playwright) that'll help us verify the changes.
Hi @adamplesnik
I've created a Playwright report comparing screenshots between develop and this branch: https://drive.sng.sk/d/s/upCsGTpy3fF0UdjPFTuo96fHfNOqRB9O/hQLWVT0_5z_yzUXQYchKpONmqhAPAkUp-nL_gZLLzrAo
Please don't be discouraged by the 'failures' it contains. We're not expecting to be perfectly matched, this is generated to give us a starting point.
I believe most of the issues can be resolved by better matching the Bootstrap and Tailwind classes. There might be some trickier issues that we'd be happy to discuss with you.
Please go over the pages and see what needs to be fixed. The report also provides a slider to help you compare the two versions:
If you give us a shout, we can generate a new report for you. Alternatively, you merge your changes into the WEBUMENIA-1998-pr-832-playwright-feature branch and then run npx playwright test to generate a new report.
@eronisko I tried to fix all major issues. Would you please run the tests again? Thank you very much.
Thanks @adamplesnik. I'll get you the test results first thing on Thursday.
New test run, looks better I think: https://drive.sng.sk/d/s/vaLZ4vSLp5IDI9nlKHPQ6Nqm6HoHXKOE/iFmH5PIBeQoKKeE3pXNF3u1uv9BuJao9-LbagJQeF0Qo