Standardize page title bottom margin styling
What does this MR do?
This MR fixes a styling bug / code debt found while working on this MR, where it was breaking the styling on /root/test1/-/wikis/PAGE/history
.
It removes the unnecessary and problematic &:last-child
from the page-title
class in typography.css
:
.page-title {
margin: #{2 * $grid-size} 0;
line-height: 1.3;
font-size: 1.25em;
font-weight: $gl-font-weight-bold;
&:last-child {
margin-bottom: 0;
}
&.with-button {
line-height: 34px;
}
}
This was originally added in this commit.
There's nothing else in the commit, and it's not clear why is was originally needed.
However, it has caused clearly caused problems over the years, and I found two unnecessary workarounds dealing with it, which were able to be deleted.
This change also fixes a couple of other places where it was causing incorrect styling which hadn't been found/addressed.
Summary of changes
- Delete the unnecessary
last-child
style - Delete two unnecessary workarounds for it.
- Removes
margin-top
from two<hr>
tags which are directly belowpage-title
elements.
Due Diligence Code Review
Before removing this, it was necessary to identify all the existing places where a .page-title:last-child
selector currently matches, and load the page in dev to make sure that the removal of the last-child
rule didn't break it.
So, I did a search of the codebase, and all the following were found via a regex search in RubyMine for page-title[^-]
with file mask !*css*,!*spec*
.
Then, I reviewed every one to see if the relevant element was the last child in the containing element, and came up with the following list, which contains the code location, path in GDK dev, and any relevant explanation.
.page-title:last-child
selector currently matches:
Existing places where a The following were found via a regex search in RubyMine for page-title[^-]
with file mask !*css*,!*spec*
.
- app/views/admin/labels/index.html.haml
- app/views/dashboard/_activity_head.html.haml
- http://gdk.test:3000/dashboard/activity
- Works because it's under .page-title-holder
- app/views/projects/blob/edit.html.haml
- http://gdk.test:3000/test-group-1/test-project-in-a-group/-/edit/master/base64d
- Removed class
.editor-title-row
fromapp/assets/stylesheets/pages/editor.scss
which was accounting for the missingmargin-bottom
.
- app/views/projects/environments/show.html.haml
- http://gdk.test:3000/test-group-1/test-project-in-a-group/-/environments/27
- Had existing spacing issue due to last-child, this MR fixes it
- app/views/projects/environments/terminal.html.haml
- http://gdk.test:3000/test-group-1/test-project-in-a-group/-/environments/27/terminal
- Had existing spacing issue due to last-child, this MR fixes it
- app/views/projects/services/_form.html.haml
- http://gdk.test:3000/test-group-1/test-project-in-a-group/-/services/emails_on_push/edit
- Header is by itself in a left column, bottom margin didn't matter
- app/views/search/show.html.haml
- Works because it's under .page-title-holder
- app/views/shared/integrations/_form.html.haml
- http://gdk.test:3000/groups/test-group-1/-/settings/integrations/emails_on_push/edit
- Header is by itself in a left column, bottom margin didn't matter
- /Users/cwoolley/workspace/gitlab-development-kit/gitlab/app/views/snippets/new.html.haml
- Works because it's under .page-title-holder
- app/views/groups/saml_providers/show.html.haml
- http://gdk.test:3000/groups/saml-group/-/saml (need to hack controller to comment all before_actions)
- These are all left-column headers, bottom margin doesn't matter
- app/assets/javascripts/feature_flags/components/edit_feature_flag.vue
- Already overrides all margins to 0
- app/assets/javascripts/whats_new/components/app.vue
- Had existing spacing issue due to last-child overriding gl-my2, this MR fixes it
- app/assets/javascripts/iterations/components/iteration_form.vue
- http://gdk.test:3000/groups/test-group-1/-/iterations/new - Need an EE license to access
- Had to remove margin top on HR underneath
- app/assets/javascripts/issuable_create/components/issuable_create_root.vue
- http://gdk.test:3000/root/test1/-/quality/test_cases/new
- Had to remove margin top on HR underneath
- app/views/admin/geo/nodes/index.html.haml
- http://gdk.test:3000/admin/geo/nodes
- Removed unnecessary class
gl-mb-5!
which was accounting for the missingmargin-bottom
.
Screenshots (strongly suggested)
Here's an example of what the Wiki history page header in this MR will look like unless we make this fix, or add yet another unnecessary workaround:
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry - [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Separation of EE specific content
Availability and Testing
- [-] Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
- [-] Tested in all supported browsers
Related Issues and MRs
- blocks !49777 (merged) (this was originally discovered due to broken styling on
/root/test1/-/wikis/PAGE/history
) - relates #288328 (closed)