commercetools / merchant-center-application-kit

Tools and components for developing Merchant Center Customizations 🛠
https://docs.commercetools.com/merchant-center-customizations
MIT License
66 stars 27 forks source link

Using Safe triangle to improve submenu navigation #3536

Open ddouglasz opened 1 month ago

ddouglasz commented 1 month ago

Summary

Using the Safe triangle pattern to improve submenu navigation

Description

Improve the submenu navigation by adding a safe area for users to smoothly navigate diagonally between the main menu and the submenu items, without awkwardly going out of focus.

Background: https://x.com/shadeed9/status/1745707615517032449?t=XnNt9gNwPHLyfrMfHEnFuA&s=03

Before: 2024-05-23 08 16 23

After (The visible triangle is only for the demo. It is not included in the PR): 2024-05-23 08 13 35

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mc-app-kit-playground ❌ Failed (Inspect) Jun 17, 2024 8:36am
merchant-center-application-kit-components-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 17, 2024 8:36am
changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: dcc992804f3196591c893461e17fc91d1a5fd3af

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

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

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

kark commented 1 month ago

Thank you @ddouglasz for working on this ❤️ I've briefly checked it in the playground and for the most part it works fine, when the cursor motion is steady. However, if I stop in the middle / change the pace of the movement, then the submenu hides.

I've noticed in the screencast from the description that the safe triangle only changes when cursor moves within the menu item boundaries. If it goes beyond the menu item, the triangle is set for good.

Screenshot 2024-05-23 at 14 41 01

In case of the implementation the triangle shrinks whenever cursor moves towards the submenu, so maybe this makes the behaviour not consistent 2024-05-23 14 31 11

ddouglasz commented 1 month ago

Thank you Kacper 🙏🏾 I will look into it and push a fix.

ddouglasz commented 1 month ago

I've noticed in the screencast from the description that the safe triangle only changes when cursor moves within the menu item boundaries. If it goes beyond the menu item, the triangle is set for good.

@kark thanks for your feedback again, after a couple of findings, I discovered a flaw with this implementation: When a user begins to navigate diagonally and then stops halfway to move to the next item, vertically, they are blocked because the triangle is set for good until we close the submenu. 2024-06-06 09 09 19

Perhaps the current implementation that allows the the safe-start point of the triangle to move is what I see is consistent with most implementations: If the users delays in the diagonal navigation, then it closes the submenu and focuses on the next item the cursor is hovering on. 2024-06-06 09 14 32

Would be nice to know what @FilPob also thinks about this 🙏🏾

kark commented 1 month ago

I've noticed in the screencast from the description that the safe triangle only changes when cursor moves within the menu item boundaries. If it goes beyond the menu item, the triangle is set for good.

@kark thanks for your feedback again, after a couple of findings, I discovered a flaw with this implementation: When a user begins to navigate diagonally and then stops halfway to move to the next item, vertically, they are blocked because the triangle is set for good until we close the submenu.

Thanks! Perhaps we should consider adding a timeout for the triangle, similar to what we can see in your MacOS screencast. 🤔

ddouglasz commented 1 month ago

I've noticed in the screencast from the description that the safe triangle only changes when cursor moves within the menu item boundaries. If it goes beyond the menu item, the triangle is set for good.

@kark thanks for your feedback again, after a couple of findings, I discovered a flaw with this implementation: When a user begins to navigate diagonally and then stops halfway to move to the next item, vertically, they are blocked because the triangle is set for good until we close the submenu.

Thanks! Perhaps we should consider adding a timeout for the triangle, similar to what we can see in your MacOS screencast. 🤔

Updated here: https://github.com/commercetools/merchant-center-application-kit/pull/3536/commits/25c5b476da0f27e4acf695003b7eb8d09869fbb3 Thank you.

gitstream-cm[bot] commented 1 month ago

This PR is missing a Jira ticket reference in the title or description. Please add a Jira ticket reference to the title or description of this PR.

gitstream-cm[bot] commented 1 month ago

🥷 Code experts: emmenko

emmenko has most 🧠 knowledge in the files.

See details `packages/application-shell/src/components/navbar/menu-items.styles.ts` Knowledge based on git-blame: `packages/application-shell/src/components/navbar/menu-items.tsx` Knowledge based on git-blame: emmenko: 58% `packages/application-shell/src/components/navbar/navbar.tsx` Knowledge based on git-blame: emmenko: 1% `packages/application-shell/src/components/navbar/use-navbar-state-manager.ts` Knowledge based on git-blame: emmenko: 63%

To learn more about /:\ gitStream - Visit our Docs