Fix the vertical rhythm

@Bojhan It's very hard to call this a major, at least in the communities definition of major.

The major priority is used for issues that are not critical, but that do have significant impact or are important by community consensus.

Compare this with... fixing a significant-but-not-critical UX issue and you'd have to say normal at best. This is a perfect candidate for a task in 8.1.x.

I think the easiest way in establishing rhythm would be to stick consistent units of vertical spacing. So based on 81.3%/1.538em, we should have units of 10px, 20px, and multiples of those.

Some elements (like paragraphs) use 1em for spacing when they should be using 10px/20px/30px/40px.

Took a stab at fixing the heading/paragraph/button spacing.

Page Titles: 15px
Breadcrumbs: 20px
Paragraphs: 10px
Headings: 20px
Form Items (except checkboxes): 20px
Checkboxes: 10px

Visual for the before/after attached.

@traci.potocnik@gmail.com Thanks for working on this issue! It seems your commit is missing the file core/themes/seven/css/components/action-links.css. Can you check if you still have this file available in your repository?

Updated patch added! The missing files are now included.

Thanks for working on this @tracipotocnik!

@Bojhan screenshot is in #10. Here's some more.

Re-roll the patch.

Verified and tested updated patch #23
patch applied successfully. thanks @BandanaSharma

Re-roll the patch 9.3.x. Kindly review.

Thanks!

Verified and tested patch #27.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: admin/content, admin/structure, admin/config
# Check the spacing between titles, tables, action buttons

Expected Results:
# User should see the accurate spacing between titles, tables, action buttons

Actual Results:
# User is not able to see the accurate spacing between titles, tables, action buttons

Can be a move to RTBC.

The CI failed for #27.

Addressing comment #30.

Tested the patch (2487027-31.patch) with core version (9.5.x-dev and 9.4.x-dev) working fine, good for RTBC.

Verified and tested patch #31.
Patch applied successfully and looks good to me.

Testing Steps:
# Goto: admin/content, admin/structure, admin/structure/views
# Check the spacing between titles, tables and action buttons

Can be a move to RTBC.

As per my previous comment patch #31 was Verified and tested.
Can be moved to RTBC.

Spotted some things to address.

1. +++ b/core/themes/seven/css/components/details.css
   @@ -24,3 +24,7 @@
   +details {
   +  margin-top: 0;
   +  margin-bottom: 20px;
   +}
   

   There's a .seven-details class a few lines up that is already getting margin styles. The margin styles should probably be changed there, or at least removed since they're immediately overridden by this.

2. +++ b/core/themes/seven/css/components/form.css
   @@ -57,7 +57,7 @@ fieldset:not(.fieldgroup) > legend {
   +  margin: 0 0 20px;
   

   I notice here and several other places that the top margin is being removed altogether. This is a pretty big change and I'm not sure it is necessary to achieve the goals of this issue since adjacent margins will collapse. If there's a good reason for this, it would be good to justify in the issue summary.

3. +++ b/core/themes/seven/css/components/form.css
   @@ -73,9 +73,17 @@ td > .form-item:only-child {
   +.form-actions {
   +  margin-top: 20px;
   +  margin-bottom: 20px;
   +}
   

   There are some other .form-actions styles further down this file. It would be best to have this adjacent to that.

4. +++ b/core/themes/seven/css/components/messages.css
   @@ -2,10 +2,10 @@
   +  margin: 0 8px 10px 0;
   

   The bottom margin is being set to 20px in LTR, I'm guessing that was intended here too?

5. +++ b/core/themes/seven/css/components/tabs.css
   @@ -82,6 +82,9 @@ li.tabs__tab a {
   +.region-header .tabs.primary {
   

   There's no guarantee the page title block is directly above the primary tabs. If the top margin is removed, this could result in an above block being directly atop the tabs with no spacing.

@bnjmnm
Corrected points mentioned by you.

Agreed with:

I notice here and several other places that the top margin is being removed altogether. This is a pretty big change and I'm not sure it is necessary to achieve the goals of this issue since adjacent margins will collapse. If there's a good reason for this, it would be good to justify in the issue summary.

So as per comment #10 replaced 0.75em as margin-top with 10px. There should be margin top.

Regarding point #3:

There are some other .form-actions styles further down this file. It would be best to have this adjacent to that.

The other styles for .form-actions are being used for narrow devices (max-width: 600px).

Regarding point #4:

The bottom margin is being set to 20px in LTR, I'm guessing that was intended here too?

Yes, that is required. And not only for LTR, it is required for RTL as well. So making that change too.
Please refer the attached screenshots for both LTR and RTL messages (before and after).

Regarding point #5:

There's no guarantee the page title block is directly above the primary tabs. If the top margin is removed, this could result in an above block being directly atop the tabs with no spacing.

I guess there is a probability of placing the Primary tabs block without any other block above it. We will need to check for such scenarios, then only we can conclude on it. So, for the time being keeping the margin: 0 for it.

Patch in #41 working fine for me on local 9.5

Verified and tested patch #41 on Drupal 9.5.x-dev. Patch applied successfully and looks good to me.

Testing steps:
1. Goto admin/Appearance
2. Set Seven theme in the Administration theme (Scroll down and see)
3. Save configuration
4. Go to Structure or Content
5. Observe the spacing above the Heading
6. Apply the patch
7. Reload the page. Now observe the spacing is displaying properly (Note: Clear all caches if required)

Testing Result:
1. After applying the patch empty space above the header is reduced

Can be moved to RTBC

Postponing on #3304285: Remove Seven from core, this should probably be moved to the contrib Seven issue queue once that issue has been committed.

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project and marking RTBC again as per #44.

The patches are for Drupal core.