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

[UserMenu] Create a new Item component that contains the user menu changes #9771

Closed
wants to merge 10 commits into from

Conversation

zakwarsame
Copy link
Contributor

@zakwarsame zakwarsame commented Jul 20, 2023

WHY are these changes introduced?

Fixes https://github.com/Shopify/polaris-summer-editions/issues/785
Fixes https://github.com/Shopify/core-workflows/issues/1029
Fixes https://github.com/Shopify/polaris-summer-editions/issues/789

  • The user menu's designs for uplift have been updated as follows:

Image

WHAT is this pull request doing?

  • This PR breaks away from the ActionList's Item being used in the Menu item. Instead we create a custom MenuItem that can easily support features like indentation and removing constraints on the avatar.
  • If a consumer wants to use this MenuItem, they provide a topSection prop that's passed to the UserMenu. The prop should specify to values: indentItems and items
  • The UserMenu passes this prop down to Menu which applies these changes by rendering the new MenuItem
  • The regular actions normally passed to UserMenu is not affected, thus anyone using UserMenu today does not need to make any changes.

A follow up to this PR is required after it's released, opened here: https://github.com/Shopify/web/pull/98734

How to 🎩

🎩 checklist

@zakwarsame
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @zakwarsame! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@zakwarsame
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @zakwarsame! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@zakwarsame zakwarsame force-pushed the zak/menu branch 2 times, most recently from 0cb50c8 to cb77783 Compare July 20, 2023 19:41
@zakwarsame zakwarsame marked this pull request as ready for review July 20, 2023 20:11
@zakwarsame
Copy link
Contributor Author

/snapit

@Shopify Shopify deleted a comment from github-actions bot Jul 20, 2023
@Shopify Shopify deleted a comment from github-actions bot Jul 20, 2023
@github-actions
Copy link
Contributor

🫰✨ Thanks @zakwarsame! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@@ -0,0 +1,46 @@
import React, {useRef, useState} from 'react';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving this component to the top-level since multiple components are now using it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll let the Polaris folk chime in but I'm not sure if this makes sense as a top level component named a such since it seems like it would be a replacement for <Text truncate />.
Maybe we can rename it to like: TextWithTooltipWhenTruncated but that's a bit wordy 😛

@zakwarsame
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @zakwarsame! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

@zakwarsame
Copy link
Contributor Author

@henryyi Please see this thread for why we're going with this approach: https://shopify.slack.com/archives/C4Y8N30KD/p1689780576017759
I think ultimately we'll want to pass in a prop to ActionList's item and make our customizaitons there. However, the approach discussed for now was to conditionally our custom store section while leaving the User Menu unaffected.

@zakwarsame
Copy link
Contributor Author

@henryyi I've refactored the code so that we pass in a topSection prop with both the indentItems and regular items pre-separated from the consumer. That way Polaris is agnostic to the admin. Thanks a lot for the feedback!

@zakwarsame
Copy link
Contributor Author

/snapit

@Shopify Shopify deleted a comment from github-actions bot Jul 21, 2023
@Shopify Shopify deleted a comment from github-actions bot Jul 21, 2023
@Shopify Shopify deleted a comment from github-actions bot Jul 21, 2023
@github-actions
Copy link
Contributor

🫰✨ Thanks @zakwarsame! Your snapshots have been published to npm.

Test the snapshots by updating your package.json with the newly published versions:

yarn add @shopify/[email protected]
yarn add @shopify/[email protected]

sam-b-rose added a commit that referenced this pull request Jul 25, 2023
#9820)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #0000 <!-- link to issue if one exists -->

Fixes Shopify/archive-polaris-backlog-2024#785
Fixes Shopify/core-workflows#1029
Fixes Shopify/archive-polaris-backlog-2024#789

<!--
  Context about the problem that’s being addressed.
-->

<details>
<summary> Expand to see specifications/designs for this PR: </summary>


![image](https://github.com/Shopify/polaris/assets/42528878/752f0a05-ffad-4361-b84e-db96cdf8ec84)

</details>



### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

- This PR takes an alternative approach to [this
PR](#9771)
- It adds a new prop to `ActionList`'s `Item` component called `indent`.
This prop adds some space and a border to the left of the individual
items.
- Given that the indentation only applies to `Plus` stores in the
original design, we can simply specify the prop in the consumer.

### How to 🎩

Non-Plus: https://admin.web.mn.zakaria-warsame.us.spin.dev/store/shop1
Plus: https://admin.web.mn.zakaria-warsame.us.spin.dev/store/shop2

Ensure the user menu matches the designs

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
 ~- [ ] Updated the component's `README.md` with documentation changes~
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Sam Rose <[email protected]>
juzser pushed a commit to juzser/polaris that referenced this pull request Jul 27, 2023
Shopify#9820)

<!--
  ☝️How to write a good PR title:
- Prefix it with [ComponentName] (if applicable), for example: [Button]
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #0000 <!-- link to issue if one exists -->

Fixes https://github.com/Shopify/polaris-summer-editions/issues/785
Fixes https://github.com/Shopify/core-workflows/issues/1029
Fixes https://github.com/Shopify/polaris-summer-editions/issues/789

<!--
  Context about the problem that’s being addressed.
-->

<details>
<summary> Expand to see specifications/designs for this PR: </summary>


![image](https://github.com/Shopify/polaris/assets/42528878/752f0a05-ffad-4361-b84e-db96cdf8ec84)

</details>



### WHAT is this pull request doing?

<!--
  Summary of the changes committed.

Before / after screenshots are appreciated for UI changes. Make sure to
include alt text that describes the screenshot.

If you include an animated gif showing your change, wrapping it in a
details tag is recommended. Gifs usually autoplay, which can cause
accessibility issues for people reviewing your PR:

    <details>
      <summary>Summary of your gif(s)</summary>
      <img src="..." alt="Description of what the gif shows">
    </details>
-->

<!-- ℹ️ Delete the following for small / trivial changes -->

- This PR takes an alternative approach to [this
PR](Shopify#9771)
- It adds a new prop to `ActionList`'s `Item` component called `indent`.
This prop adds some space and a border to the left of the individual
items.
- Given that the indentation only applies to `Plus` stores in the
original design, we can simply specify the prop in the consumer.

### How to 🎩

Non-Plus: https://admin.web.mn.zakaria-warsame.us.spin.dev/store/shop1
Plus: https://admin.web.mn.zakaria-warsame.us.spin.dev/store/shop2

Ensure the user menu matches the designs

### 🎩 checklist

- [x] Tested on
[mobile](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting.md#cross-browser-testing)
- [x] Tested on [multiple
browsers](https://help.shopify.com/en/manual/shopify-admin/supported-browsers)
- [x] Tested for
[accessibility](https://github.com/Shopify/polaris/blob/main/documentation/Accessibility%20testing.md)
 ~- [ ] Updated the component's `README.md` with documentation changes~
- [x] [Tophatted
documentation](https://github.com/Shopify/polaris/blob/main/documentation/Tophatting%20documentation.md)
changes in the style guide

---------

Co-authored-by: Sam Rose <[email protected]>
Copy link
Contributor

Hi! We noticed there hasn’t been activity on this PR in a while. After 30 days, it will close automatically.

If it’s still relevant, or you have updates, comment and let us know. And don’t worry, you can always re-open later if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants