Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Follow-up to #1342054: [META] Clean up templates and CSS
Problem/Motivation
- Bartik's template files need to be assessed and cleaned up of redundant markup, bad formatting and ID's.
- Bartik's CSS files need to follow Drupal's CSS Coding Standards.
Proposed resolution
For this issue we take "views.css" within Bartik in css/components/views.css plus any template file associated with the CSS and clean them up.
Formatting tasks to do
- Remove blank lines where necessary - guidelines are here.
- The CSS file needs to have the correct Comment formats - see guidelines here and also reference other fixed Bartik CSS files for wording guidelines.
CSS cleanup tasks to do
- Remove any CSS that repeats styles from elsewhere in Bartik and therefore are not needed in this file.
- Check all of the selector rules are correct and are currently in use on the frontend of Bartik.
Remaining tasks
- Write a patch containing as much as the above as possible.
- Post a patch with screenshots.
- Visual review of a patch - check the elements being styles visually, with and without patch applied. Take screenshots.
- Code review of a patch - check the code follows coding standards, suggest improvements if needed in a comment.
- Produce a new patch with improvements if needed.
User interface changes
None
API changes
None
Data model changes
None
Beta phase evaluation
Issue category | Task because it is refactoring CSS and templates in Bartik |
---|---|
Issue priority | Not critical because Bartik functions fine we are just doing cleanup tasks |
Unfrozen changes | Unfrozen because it only changes CSS and markup |
Prioritized changes | The main goal of this issue is usability of the Bartik's codebase |
Disruption | non-Disruptive as it is just changing markup and CSS |
Comment | File | Size | Author |
---|---|---|---|
#8 | Screenshot 2015-09-01 18.06.56.jpg | 30.35 KB | LewisNyman |
#8 | Screenshot 2015-09-01 18.06.24.jpg | 50.28 KB | LewisNyman |
#7 | clean_up_the_views-2542600-7.patch | 3.2 KB | emma.maria |
#7 | interdiff-6-7.txt | 355 bytes | emma.maria |
#6 | clean_up_the_views-2542600-6.patch | 3.2 KB | emma.maria |
Comments
Comment #1
Manjit.SinghComment #2
Manjit.SinghComment #3
emma.mariaI noticed that there was a lot of redundant CSS code, selectors that could be improved, a lot of code that was being overwritten by the Views module that I made kick in / got rid of as it was not needed, and declarations of styles to improve.
As the list of improvements to make was going to be a huge list I knocked up a patch myself.
Patch and interdiff attached.
Comment #4
emma.mariaBecause the changes are big I have attached a visual regression report to show what's going on in the frontend.
There is only one difference which is an improvement.
Before
After
A code review is much needed please :)
Comment #5
LewisNymanGreat, I can confirm that there are no visual regressions. Just a few questions:
Why are we disabling this styling for a summary in this particular occasion? It feels inconsistent for the sake of it.
This doesn't work as expected, as the styling is directly overriding from
.region-content ul, .region-content ol
. Usually in these situations we have to set the padding again but I would just remove all padding so you don't have to set it again for RTL.Comment #6
emma.mariaI agree with the first point. Bartik declares the summary styling so we shouldn't override it just for Views.
Also good spot on the padding of
.views-displays .tabs .action-list
. I wondered what was going on there,padding: 0
creates no visual regressions.I have made the changes in this patch.
PS. should we capitalise 'Views' in the file comment?
Comment #7
emma.mariaI added the capital letter to "Views" in the file comment. All other feedback has been addressed. Can someone please final check this for me?
Comment #8
LewisNymanGreat, thanks for making these changes. I can confirm there are no regressions.
Comment #11
rudraram CreditAttribution: rudraram at Axelerant commentedComment #12
rudraram CreditAttribution: rudraram at Axelerant commentedRe-test passed so RTBC.
Comment #13
alexpottCommitted 2c62403 and pushed to 8.0.x. Thanks!