Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix "Marketing & Merchandising" padding on store management panel #38088

Merged
merged 4 commits into from May 10, 2023
Merged

Fix "Marketing & Merchandising" padding on store management panel #38088

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a73e1d
Add a 24px spacing between the Manage my store header and the Marketi…
chihsuan May 3, 2023
92a1858
Add changelog
chihsuan May 3, 2023
30aab39
Update store management card style
chihsuan May 8, 2023
e4af20d
Update .woocommerce-store-management-links__card-body padding
chihsuan May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions plugins/woocommerce-admin/client/store-management-links/style.scss
View file
Open in desktop
Expand Up @@ -3,3 +3,7 @@
.woocommerce-store-management-links__card-body.is-size-custom {
padding: 24px 0 8px 0;
}

.woocommerce-store-management-links__card-body {
padding-top: 24px;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, @chihsuan! However, I'm seeing the 24px padding at the top is not symmetrical with the 16px (or $gap) that's used for the typical vertical spacing around widget children.

Suggested change
padding-top: 24px;
padding-top: $gap;

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye! @ilyasfoo What do you think if we use padding: $gap-large 0 $gap-smaller 0; (padding: 24px 0 8px 0;)?

It looks like it was supposed to be like that.

woocommerce/plugins/woocommerce-admin/client/store-management-links/style.scss

Lines 2 to 5 in 92a1858

// We need to do this to allow item hover backgrounds to stretch the whole card body.
.woocommerce-store-management-links__card-body.is-size-custom {
padding: 24px 0 8px 0;
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just made the changes first in 30aab39!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, however, I'm seeing the padding that's added gives a "border-like" artefact at the bottom when hovering the last item:

image

image

Whereas, in inbox notes items, tasklist items do not have this.

But perhaps I may be overly complicating it. I don't know if we need to call our expert @verofasulo for opinion 🙏 😄

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing the padding that's added gives a "border-like" artefact at the bottom when hovering the last item

Good catch, @ilyasfoo! I agree that it would be better to remove the padding there to avoid that effect.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @verofasulo @ilyasfoo! Updated as suggested in e4af20d. 🙌

}
4 changes: 4 additions & 0 deletions plugins/woocommerce/changelog/fix-manage-store-panel-padding
View file
Open in desktop
@@ -0,0 +1,4 @@
Significance: patch
Type: tweak

Add a 24px spacing to store management body