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.

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

Design Picker: Update filter styles #97393

Merged
merged 4 commits into from
Dec 13, 2024

Conversation

arthur791004
Copy link
Contributor

@arthur791004 arthur791004 commented Dec 12, 2024

Related to https://github.com/Automattic/dotcom-forge/issues/10108

Proposed Changes

  • Update styles of the filters
Before After
image image
image image

Why are these changes being made?

Testing Instructions

  • Create a new site
  • On the Goals screen, select any goals
  • On the Design Picker screen, select/deselect any categories
  • Ensure new styles look good
  • Disable the feature flag by adding flags=-design-picker/goal-centric to the end of the URL
  • Ensure styles still look good

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@arthur791004 arthur791004 self-assigned this Dec 12, 2024
@arthur791004 arthur791004 requested review from fditrapani and a team December 12, 2024 09:38
@matticbot matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 12, 2024
@arthur791004 arthur791004 force-pushed the feat/design-picker-filter-styles branch from 9c34ee3 to f71d17e Compare December 12, 2024 09:40
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Copy link
Contributor

@fditrapani fditrapani left a comment

Choose a reason for hiding this comment

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

Nice work! Tested the front-end only. This looks great. I like how you handled the mobile.

This can be handled separately but we might need to do some tweaking around when we drop down to two columns because there are some breakpoints where things are getting cut off (Around 900px):

image

@arthur791004 arthur791004 force-pushed the feat/design-picker-filter-styles branch from f71d17e to c0079a9 Compare December 13, 2024 03:41
@arthur791004
Copy link
Contributor Author

This can be handled separately but we might need to do some tweaking around when we drop down to two columns because there are some breakpoints where things are getting cut off (Around 900px):

I've adjusted the breakpoint to 782px push it to the next row!

Copy link
Contributor

@taipeicoder taipeicoder left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@madhusudhand madhusudhand left a comment

Choose a reason for hiding this comment

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

There is about 2px difference in the top alignment of the two filter groups.

image

Everything else looks good.

@arthur791004
Copy link
Contributor Author

There is about 2px difference in the top alignment of the two filter groups.

Nice catch! Fixed by c1fa1b1.

Copy link
Member

@madhusudhand madhusudhand left a comment

Choose a reason for hiding this comment

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

LGTM! 🚢

@arthur791004
Copy link
Contributor Author

Nice catch! Fixed by c1fa1b1.

Hmmmm...it made the behavior of the responsive toolbar weird...

@matticbot
Copy link
Contributor

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • notifications

To test WordPress.com changes, run install-plugin.sh $pluginSlug feat/design-picker-filter-styles on your sandbox.

@arthur791004 arthur791004 merged commit a4407d2 into trunk Dec 13, 2024
9 of 11 checks passed
@arthur791004 arthur791004 deleted the feat/design-picker-filter-styles branch December 13, 2024 06:03
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Dec 13, 2024
timur987 pushed a commit that referenced this pull request Dec 13, 2024
* Design Picker: Update filter styles

* Fix styles on single selection

* Remove border from the topic list

* Fix the behavior of the responsive toolbar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants