Misc design refinements

[sofiamatulis]

Why are these changes introduced?

Fixes #442 .

What approach did you take?

For the spacing update for when we have the setting of 1600 I decided to add the 2remin the calculation. This way, if we do update the value we dont have to update it everywhere. For now I updated it only on base.css but I believe some of the margin and padding is set on the section level so I will need to update it there as well

For the other misc items with questions/comments I have added comments directly below

Other considerations

Demo links

• Editor

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[ludoboludo]

First look, left a few comments.

[ludoboludo]

Started reviewing again but I will finish tomorrow morning 👍

[sofiamatulis]

Here are the current patterns for 1200 (for 1600 youd add or remove 2rem depending if its negative or positive since its from 5rem to 7rem)

Mobile and Desktop:
padding-top: 4rem; (for 1600 it would be 6rem) Mobile will always default to the 1200 values

1. .blog.background-secondary
2. multicolumn.background-secondary
3. .collection-hero.collection-hero--with-image

padding-bottom: 5rem; (for 1600 it would be 7rem)

1. .background-secondary
2. .blog.background-secondary
3. .multicolumn.background-secondary
4. .multicolumn:not(.background-secondary)
5. .image-with-text:not(.color-scheme-background-1)

Mobile
margin-top: -1rem; (ONLY FOR 1200)

1. .collage-wrapper-title
2. .collection-list-wrapper:not(.no-heading) .title-wrapper-with-link
3. .blog:not(.no-heading)
4. .collection .title:not(.title--no-heading)

Desktop
margin-top: 6rem; (for 1200, so for 1600 it would be 8rem)

1. .collage-wrapper.no-heading (ONLY FOR 1200)
2. .collection-list-wrapper.no-heading
3. .blog.no-heading
4. .multicolumn.no-heading:not(.background-secondary)
5. .collection .title-wrapper-with-link--no-heading

Some other rules:

1. When there are multiple collage sections without titles they stack under each other: https://screenshot.click/10-34-sqi0t-jmux0.mp4

2. The sections that have product cards (featured collections, search results) should have a reduce margin-bottom, because there is additional spacing below each product card. This is where you will see the class negative-margin. Video: https://screenshot.click/10-35-gqebt-oin3l.mp4 You will find it being used in featured collections, search results, main collection and collection list.

3. The collage section and collection stack (in this order). When collection list doesn't have a title: https://screenshot.click/10-37-2ioz7-e3kw3.mp4

4)The email signup doesn't have a margin-bottom when above the footer (last section) when background one and full width is selected. Otherwise youd end up with double the margin.

5. Email signup and rich text have less spacing below when background 1 is selected in the colour setting.

6. When titles are added to sections the margin-top should be 5rem on desktop and 4rem on mobile (for 1200). When titles are removed, the margin-top should be 6rem on desktop and 5rem on mobile. This only applies to: featured collection, collection list, collage, multi-column, blog posts, page section.

[Oliviammarcello]

@sofiamatulis Looks great, just have a few notes:

• The spacing between the section title and the section is now much larger when it shouldn't be. What is done in the live version should remain. Current | This PR
• For the pagination, the hover over numbers is a 2px thick line, can it be changed to 1px. Also, the arrow still has an opacity. Video
• The spinner in the add to cart button and on the cart a much larger than the spinner on the filtering page. Can we decrease the size of the first two spinners to match what is done on filters?
• The margins between sections have been updated on desktop and mobile. It should only be updated when the screen width is set to 1600px (Wide setting) on desktop not mobile.
• For the collage section, there is a very faint line on the collection cards still. Screenshot.

cc @wiktoriaswiecicka

[sofiamatulis]

• For the collage section, there is a very faint line on the collection cards still. Screenshot.

@Oliviammarcello Thanks for the feedback! For the collage cards, I cant seem to reproduce what you see 🤔 I already have it set to have no border: https://screenshot.click/10-26-lkn0e-jevrh.mp4

Do you mind sharing what collection youre adding and re-trying it?

I can't seem to reproduce it either now 🤔 Not too sure what happened

[martinamarien]

I haven't reviewed everything. But while I take a break, I thought I'd leave these comments for now.

[Oliviammarcello]

• On mobile there is still one issue where the spacing is too tight under the rich text cta. Video.

[Oliviammarcello]

• I'm not sure if this broke in this PR, but the collage section layout with 1 or 2 blocks is now not working. Video

This may have happened when fixing the collage, collection list, and multicolumn sections inconsistent grid alignments?

[Oliviammarcello]

LGTM🙌

[ludoboludo]

LGTM as well 👍

[KaichenWang]

Looks good!

Two other devs have reviewed.