refactor(custom views selector): ui refinement #3308

kark · 2023-11-24T08:39:05Z

Summary

This PR changes the UI of the custom views selector component.
It doesn't cover the correct placement of the selector.

Screen.Recording.2023-11-27.at.10.39.05.mov

The changes can be observed in visual regression tests output 👇.

Open questions:

designs	PR

for now I haven't changed the icon and the title.

Regarding the icon, it felt to me it is associated with an additional feature for hiding the selector upon clicking the icon. However, as of now, this functionality has not been implemented.
As for the title, is it essential to remove the colon? Please note that if the colon is removed, all messages will need to be retranslated.

changeset-bot · 2023-11-24T08:39:09Z

🦋 Changeset detected

Latest commit: dd02d2b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 38 packages

Name	Type
@commercetools-website/components-playground	Patch
@commercetools-frontend/mc-html-template	Patch
@commercetools-local/visual-testing-app	Patch
@commercetools-frontend/application-components	Patch
@commercetools-frontend/mc-scripts	Patch
@commercetools-frontend/application-shell	Patch
@commercetools-frontend/codemod	Patch
@commercetools-applications/merchant-center-template-starter-typescript	Patch
@commercetools-applications/merchant-center-template-starter	Patch
@commercetools-applications/merchant-center-custom-view-template-starter-typescript	Patch
@commercetools-applications/merchant-center-custom-view-template-starter	Patch
@commercetools-local/playground	Patch
@commercetools-frontend/cypress	Patch
@commercetools-backend/eslint-config-node	Patch
@commercetools-backend/express	Patch
@commercetools-backend/loggers	Patch
@commercetools-frontend/actions-global	Patch
@commercetools-frontend/application-config	Patch
@commercetools-frontend/application-shell-connectors	Patch
@commercetools-frontend/assets	Patch
@commercetools-frontend/babel-preset-mc-app	Patch
@commercetools-frontend/browser-history	Patch
@commercetools-frontend/constants	Patch
@commercetools-frontend/create-mc-app	Patch
@commercetools-frontend/eslint-config-mc-app	Patch
@commercetools-frontend/i18n	Patch
@commercetools-frontend/jest-preset-mc-app	Patch
@commercetools-frontend/jest-stylelint-runner	Patch
@commercetools-frontend/l10n	Patch
@commercetools-frontend/mc-dev-authentication	Patch
@commercetools-frontend/notifications	Patch
@commercetools-frontend/permissions	Patch
@commercetools-frontend/react-notifications	Patch
@commercetools-frontend/sdk	Patch
@commercetools-frontend/sentry	Patch
@commercetools-frontend/url-utils	Patch
@commercetools-website/custom-applications	Patch
@commercetools-website/custom-views	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

swarmia · 2023-11-24T08:39:09Z

✅ Linked to Task SHIELD-968 · Update Custom Views selector UI
➡️ Part of Epic SHIELD-938 · Custom Views preparation for Closed Beta

github-actions · 2023-11-24T08:42:38Z

Deploy preview for application-kit-custom-views ready!

✅ Preview
https://application-kit-custom-views-4wt902ee7-commercetools.vercel.app
https://appkit-cv-sha-1a8798469b5c5b6fdc3f2a99573e15f108064daf.commercetools.vercel.app
https://appkit-cv-pr-3308.commercetools.vercel.app

Built with commit dd02d2b.
This pull request is being automatically deployed with vercel-action

github-actions · 2023-11-24T08:42:43Z

Deploy preview for merchant-center-application-kit ready!

✅ Preview
https://merchant-center-application-a4hnlhyo4-commercetools.vercel.app
https://appkit-sha-1a8798469b5c5b6fdc3f2a99573e15f108064daf.commercetools.vercel.app
https://appkit-pr-3308.commercetools.vercel.app

Built with commit dd02d2b.
This pull request is being automatically deployed with vercel-action

emmenko

Nice work! 🤗

emmenko · 2023-11-28T08:43:51Z

...ation-components/src/components/custom-views/custom-views-selector/custom-views-selector.tsx

+      ${designTokens.colorNeutral98},
+      ${designTokens.colorNeutral98}
+    );
+  padding: 7px ${designTokens.spacing20};


Nit: why 7? Can't we use the normal scale of multiple of 4s (4, 8, 12, 16, ...)?

Sounds good to me 👍, would it be ok with you @NataliaSclipet?

emmenko · 2023-11-28T08:47:23Z

...ation-components/src/components/custom-views/custom-views-selector/custom-views-selector.tsx

+        display: flex;
+        align-items: center;
+        padding: ${designTokens.spacing20} ${designTokens.spacing25};
+        height: 30px;


Nit: also here, why do we need to make an exception for this?

Can't we use the design token for buttons (medium)?

https://github.com/commercetools/ui-kit/blob/dc24278e712b45a08872cbf3608a3d36dabe74d0/design-system/materials/internals/definition.yaml#L971-L972

Sounds good, just double checking with @NataliaSclipet

Can we keep this exception both for here and above please? I'm aware we are not using our design tokens, however these exceptions here has been defined and reviewed together with Sven and Simone. @kark

Thank you, that's fine with me 😊

emmenko · 2023-11-28T08:48:17Z

...ation-components/src/components/custom-views/custom-views-selector/custom-views-selector.tsx

+  width: 1px;
+  height: 18px;
+  background-color: #cccccc;


Nit: these values shouldn't be hardcoded

Appreciate it! I discovered this color hardcoded in the UI kit, so I left it hardcoded here as well.
https://github.com/commercetools/ui-kit/blob/b8ed8e4f527c69c18ab5c6f536a1b0cbffa92eea/design-system/materials/internals/story/shared-components.js#L19-L22
But now I've noticed that's only used in a story.
I can add a design token 👍

Don't we have an existing design token for this color? Why do we need to add a new one?

If I'm not mistaken, #cccccc translates to hsl(0, 0%, 80%). I don't think we have it available as a token.

NataliaSclipet · 2023-11-28T09:10:24Z

Thank you for working on it @kark .
Regarding the title and the icon, could you implement the title and icon that are in the new designs and keep the colon, (don't remove it), please?
Thank you for sharing your input regarding the icon, the decision of changing it comes from Simone and Sven.

CarlosCortizasCT · 2023-11-28T11:41:33Z

Regarding the Custom Views label, please also be aware the design is using font-weigth: 300 and we're not importing that value for the Inter font (reference).

(we would also need to include it in the Vistual Testing app)

CarlosCortizasCT

Looks great to me 💯

Thanks a lot for working on this one, Kacper 🙇

emmenko

Thanks! 🙌

As discussed in our sync let's keep these special changes as a (hopefully) temporary implementation.

chore: changeset test: fix refactor: use accessiblebutton refactor: use text detail style: add border radius style: use gradient for the background style: use correct colors

CarlosCortizasCT · 2023-11-30T08:33:03Z

@NataliaSclipet Here we are only changing the UI of the selector but we have another PR where we also change its placement on the different pages.

I would suggest you make your review in the other PR (after this one is merged and the other one gets this changes) instead so you get the full picture and, if there's any styling issue with the selector, I can adjust it over there.

What do you think?

kark · 2023-11-30T08:51:06Z

@NataliaSclipet Here we are only changing the UI of the selector but we have another PR where we also change its placement on the different pages.

I would suggest you make your review in the other PR (after this one is merged and the other one gets this changes) instead so you get the full picture and, if there's any styling issue with the selector, I can adjust it over there.

What do you think?

We have just discussed that with @NataliaSclipet to do what @CarlosCortizasCT suggested, so I'll merge this PR now 🙂

github-actions bot deployed to Preview November 24, 2023 08:41 View deployment

github-actions bot deployed to Preview November 24, 2023 09:00 View deployment

kark force-pushed the SHIELD-968-custom-views-selector branch from 59f1a29 to 72a7d0e Compare November 27, 2023 09:21

github-actions bot deployed to Preview November 27, 2023 09:24 View deployment

github-actions bot deployed to Preview November 27, 2023 09:45 View deployment

github-actions bot deployed to Preview November 27, 2023 10:05 View deployment

kark force-pushed the SHIELD-968-custom-views-selector branch from 4e03bf0 to 69cacb8 Compare November 27, 2023 10:09

github-actions bot deployed to Preview November 27, 2023 10:11 View deployment

github-actions bot deployed to Preview November 27, 2023 10:14 View deployment

kark marked this pull request as ready for review November 27, 2023 10:21

kark requested review from NataliaSclipet and a team November 27, 2023 10:21

CarlosCortizasCT mentioned this pull request Nov 27, 2023

Change Custom Views selector positioning #3312

Merged

emmenko reviewed Nov 28, 2023

View reviewed changes

github-actions bot deployed to Preview November 28, 2023 16:29 View deployment

github-actions bot deployed to Preview November 28, 2023 16:33 View deployment

github-actions bot deployed to Preview November 28, 2023 16:34 View deployment

kark force-pushed the SHIELD-968-custom-views-selector branch from b034294 to 877266a Compare November 29, 2023 08:42

github-actions bot deployed to Preview November 29, 2023 08:44 View deployment

github-actions bot deployed to Preview November 29, 2023 08:45 View deployment

github-actions bot deployed to Preview November 29, 2023 15:09 View deployment

CarlosCortizasCT approved these changes Nov 29, 2023

View reviewed changes

emmenko approved these changes Nov 29, 2023

View reviewed changes

kark added 5 commits November 30, 2023 08:50

refactor(customviewselector): ui refinement

d4f1d21

chore: changeset test: fix refactor: use accessiblebutton refactor: use text detail style: add border radius style: use gradient for the background style: use correct colors

refactor: cleanup

5cbe296

style: use Inter weight 300

73972d1

chore: another changeset

9cb741b

refactor: use new icon

dd02d2b

kark force-pushed the SHIELD-968-custom-views-selector branch from d4e4d39 to dd02d2b Compare November 30, 2023 07:50

github-actions bot deployed to Preview November 30, 2023 07:52 View deployment

kark assigned kark and unassigned kark Nov 30, 2023

kark merged commit 4662db5 into main Nov 30, 2023
20 checks passed

kark deleted the SHIELD-968-custom-views-selector branch November 30, 2023 08:52

ct-changesets bot mentioned this pull request Nov 30, 2023

Version Packages #3309

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(custom views selector): ui refinement #3308

refactor(custom views selector): ui refinement #3308

kark commented Nov 24, 2023 •

edited

Loading

changeset-bot bot commented Nov 24, 2023 •

edited

Loading

swarmia bot commented Nov 24, 2023

github-actions bot commented Nov 24, 2023 •

edited

Loading

github-actions bot commented Nov 24, 2023 •

edited

Loading

emmenko left a comment

emmenko Nov 28, 2023

kark Nov 28, 2023

emmenko Nov 28, 2023

kark Nov 28, 2023

NataliaSclipet Nov 28, 2023 •

edited

Loading

kark Nov 28, 2023

emmenko Nov 28, 2023

kark Nov 28, 2023 •

edited

Loading

emmenko Nov 28, 2023

kark Nov 28, 2023

NataliaSclipet commented Nov 28, 2023

CarlosCortizasCT commented Nov 28, 2023

CarlosCortizasCT left a comment

emmenko left a comment

CarlosCortizasCT commented Nov 30, 2023

kark commented Nov 30, 2023

refactor(custom views selector): ui refinement #3308

refactor(custom views selector): ui refinement #3308

Conversation

kark commented Nov 24, 2023 • edited Loading

Summary

changeset-bot bot commented Nov 24, 2023 • edited Loading

🦋 Changeset detected

swarmia bot commented Nov 24, 2023

github-actions bot commented Nov 24, 2023 • edited Loading

github-actions bot commented Nov 24, 2023 • edited Loading

emmenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NataliaSclipet Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kark Nov 28, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NataliaSclipet commented Nov 28, 2023

CarlosCortizasCT commented Nov 28, 2023

CarlosCortizasCT left a comment

Choose a reason for hiding this comment

emmenko left a comment

Choose a reason for hiding this comment

CarlosCortizasCT commented Nov 30, 2023

kark commented Nov 30, 2023

kark commented Nov 24, 2023 •

edited

Loading

changeset-bot bot commented Nov 24, 2023 •

edited

Loading

github-actions bot commented Nov 24, 2023 •

edited

Loading

github-actions bot commented Nov 24, 2023 •

edited

Loading

NataliaSclipet Nov 28, 2023 •

edited

Loading

kark Nov 28, 2023 •

edited

Loading