ActionsList filtering #9907

MaxCloutier · 2023-08-09T12:53:50Z

Why?

With extension GA comes a lot of potential new items in the More Actions of pages supporting extensibility making it increasingly more difficult for merchants to find what they are looking for.
The median number of links in "More action" menus on the products, orders, and customers pages for high GMV merchants is already between 12 and 31. This number does not count any non-app actions that have been added, so in reality, the number of actions is even higher. This is a terrible UX for our merchants.

Proposed Changes

Adding a search input that will appear anytime an ActionList has more than 10 elements.

This will give us the filtering behaviour in various areas and cases for free:

When actionGroups are rolledUp
In Bulk Actions
When actionGroups are not rolledUp, but still have more than 10 items

This will also enable the rest of the admin to leverage this behaviour for free to give a consistent merchant experience.

Open questions

Using the TextField (or SearchField from filters) creates a circular dependency with the ActionList. What would be the right approach to have the search input with an icon without creating circular dependencies?

Resources

Action Extensions tech doc - https://docs.google.com/document/d/1BEAHpkfeyUTaxW3B18LjojimDmqNLr1Fc2wfpMXzQ64/edit
Mockups for Action Extensions - https://www.figma.com/proto/afrUykUTB0n4GB4evE5F6R/%F0%9F%92%8EAdmin-UI-extension-patterns?node-id=398-111475&scaling=scale-down&page-id=295%3A118673&starting-point-node-id=398%3A111475&show-proto-sidebar=1
Post in Admin UX (WxM) with an initial proposal - https://shopify.workplace.com/groups/1294395484626415/permalink/1321299791935984/
Investigation of changes needed - https://github.com/Shopify/app-ui/issues/36#issuecomment-1468527331

translation-platform · 2023-08-11T14:17:23Z

polaris-react/locales/en.json

@@ -8,6 +8,13 @@
        "rollupButton": "View actions"
      }
    },
+    "ActionList": {
+      "SearchField": {
+        "clearButtonLabel": "Clear",


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value Clear for key Polaris.ActionList.SearchField.clearButtonLabel is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

translation-platform · 2023-08-11T14:17:24Z

polaris-react/locales/en.json

+    "ActionList": {
+      "SearchField": {
+        "clearButtonLabel": "Clear",
+        "search": "Search",


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value Search for key Polaris.ActionList.SearchField.search is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

translation-platform · 2023-08-11T14:17:25Z

polaris-react/locales/en.json

+      "SearchField": {
+        "clearButtonLabel": "Clear",
+        "search": "Search",
+        "placeholder": "Search actions"


Localization quality issue found

The following issues may affect the quality of localized translations if they are not addressed:

The value Search actions for key Polaris.ActionList.SearchField.placeholder is very short. Short strings are more likely to be misunderstood by translators without context. Please provide additional context for the translators if possible.

Please look out for other instances of this issue in your PR and fix them as well if possible.

Questions about these messages? Hop in the #help-localization Slack channel.

elanalynn

🎩'd. This looks great Max! I'm also curious about the circular dependency. I don't see anything that would cause it here. What am I missing?

elanalynn · 2023-08-11T20:56:17Z

polaris-react/src/components/ActionList/components/SearchField/SearchField.scss

+  background-color: var(--p-color-bg);
+  border: var(--p-border-width-1) solid var(--p-color-border-input);
+  // stylelint-disable-next-line -- hard coded to address accessbility issue https://github.com/Shopify/polaris/issues/7838
+  border-top-color: #898f94;


Is there no token for this color?

I started from the SearchField.scss from the topbar since Textfield and SearchField were both giving me circular dependencies issues. It was using that color 🤷

sam-b-rose · 2023-08-15T11:28:54Z

polaris-react/src/components/ActionList/ActionList.tsx

+    return totalSectionItems;
+  }, [filteredSections]);
+
+  const showSearch = totalActions >= 10;


I do like how this doesn't add a prop and then enforces this UX pattern consistently. This would affect current usages where total actions are greater than or equal to 10, though I'd bet that is a very low amount.

sam-b-rose · 2023-08-15T11:30:33Z

polaris-react/src/components/ActionList/ActionList.tsx

+    <>
+      {showSearch && (
+        <Box padding="2" paddingBlockEnd={totalFilteredActions > 0 ? '0' : '2'}>
+          <SearchField


Do we have a generic text input component that could be leveraged? Seems like we shouldn't need to recreate a basic input for the ActionList specifically 🤔

TextField and SearchField were both giving me circular dependency issues because one or more of their child components is using ActionList. They also didn't have the magnifying glass icon or the option to add it. The closest was the Topbar SearchField, but it was not meant to be reusable and the style wasn't quite what was needed either unfortunately

sam-b-rose · 2023-08-16T19:38:32Z

Hi @MaxCloutier! Would the current Listbox with search pattern be suitable? (Codesandbox example)

Maybe I'm missing context here, but looking over #8686, seems like Searchable Listbox is what is described.

MaxCloutier · 2023-08-18T13:13:01Z

Hi @MaxCloutier! Would the current Listbox with search pattern be suitable? (Codesandbox example)

Maybe I'm missing context here, but looking over #8686, seems like Searchable Listbox is what is described.

Hey @samrose3 it's quite exactly what we would need, but I believe the problem we had with this approach originally both our teams don't have the capacity to do the changes everywhere it would need to be used. Adding the search field to the ActionList covers all of the scenarios we have for "free" since it's used in bulk actions, page action groups and rolled up actions... I think these areas might evolve into using Listbox eventually, but for now it was beyond the scope of what we needed.

sam-b-rose · 2023-08-18T19:01:32Z

Very much understand. Adding this search feature here has higher leverage for getting this functionality across the system.

My only concerns are the addition of duplicate components (SearchField/TextField) and the design assumption of adding the filter after 10 items.

Looking deeper into the circular dependency issue, looks like it comes from adding the connectedDisclosure prop to the Button component, which is used in TextField. There is only one instance of that being used in the admin.

I am thinking this would be a good opportunity to deprecate that prop to remove the circular dependency and then we could re-use the TextField here.

An alternative I considered would be providing a slot would allow the TextField to be passed in and action filtering could be done outside of the component. Though, I think that strategy would run into the same manual update challenges as using the Listbox with Search.

Proposal

Could we open a PR to remove the circular dependency by removing the ActionList from Button used in connectedDisclosure?
Then can we re-use the TextField here
Consider using a prop or making the filter function controlled externally to avoid committing to the assumed rule of displaying when 10 or more items

Just trying to think for the long term here even though I realize this work solved the immediate need now. Open to push back or alternatives on this proposal 😄

MaxCloutier · 2023-08-22T13:38:32Z

@samrose3

Consider using a prop or making the filter function controlled externally to avoid committing to the assumed rule of displaying when 10 or more items

I've thought of this, but it makes things a lot more complicated to use since the places were we need this filtering behaviour, the ActionList is nested under multiple components (i.e. Page > Header > ActionMenu > RollupActions || Actions > ActionList). I think there will be a time where we'll want to offer the possibility to offer a custom filtering method, but for now it's far beyond our need and would require a much deeper look into various areas of Polaris which is out of scope for us right now.

Could we open a PR to remove the circular dependency by removing the ActionList from Button used in connectedDisclosure?

I don't know how Polaris usually deprecates props/functionalities, but I thought anything Dev facing had to be supported for at least 1 year after deprecation 🤔 If that's the case, we can't really go this way.

Then can we re-use the TextField here

I couldn't find a way to have the clear button and/or the search icon while using the TextField when I was testing initially, I might've missed something, do you know what props it would be?

sam-b-rose · 2023-08-23T14:08:56Z

anything Dev facing had to be supported for at least 1 year after deprecation

I don't think this is a constraint we have. We are looking into deprecating connectedDisclosure in the upcoming v12 release.

To minimize duplication and keep the system simple, I'd like to propose we clean up this circular dependency first and ensure the TextField is flexible enough for achieving this search input pattern 😄 this way we can compose search features more seamlessly in the future for other components as well

@kyledurand has already cleaned up the connectedDisclosure going into the next branch and will mark it as deprecated in main 👏

MaxCloutier · 2023-08-23T14:17:03Z

@samrose3 Could we prioritize @kyledurand's PR so it can be merged in next asap so I can rebase this branch with next and add the missing props/styles to the TextField?

kyledurand

I'm pretty against adding all of this duplication and complexity to the system. > 600 lines of code that's adding a custom input, not even using the Polaris TextField just feels pretty wrong to me.

Polaris is meant to be composable pieces of UI and we ship patterns using the styleguide. The listbox with search is what should be used in your case. Can you create this composition in another central location and use it everywhere its needed?

MaxCloutier · 2023-08-23T15:56:47Z

I'm pretty against adding all of this duplication and complexity to the system. > 600 lines of code that's adding a custom input, not even using the Polaris TextField just feels pretty wrong to me.

Polaris is meant to be composable pieces of UI and we ship patterns using the styleguide. The listbox with search is what should be used in your case. Can you create this composition in another central location and use it everywhere its needed?

@kyledurand if we do what @samrose3 is suggesting and use TextField after the circular dependency is resolved by your PR it would get rid of the search input duplication code and take this current change to ~300 lines of code, 100 of which is tests and another 50 is probably translations. Would that be more acceptable to you?

My biggest concern with replacing ActionList with listbox with search is that I would likely have to change everywhere that the ActionList is currently being used in Polaris to keep the styling consistent across components. On top of that, that implementation looks more complex than the implementation in this PR so I'm not sure we would benefit from it.

As a reminder, the reason why this is implemented in the ActionList is because we need this functionality in components where the ActionList is not controlled by consumers of Polaris (i.e. Page rendering actionGroups, IndexTable rendering bulkActions, etc.)

sam-b-rose · 2023-08-23T16:40:46Z

we need this functionality in components where the ActionList is not controlled by consumers of Polaris

I see now. Yeah, that limitation is certainly a blocker to the Listbox strategy.

Thank you for your patience as I've continued to build my understanding of the limitations and constraints, @MaxCloutier! Let's get the connectedDisclosure PR merged and try the component re-use approach.

Sorry for blocking this. I know this is impacting our merchant UX and want to resolve a smart systematic approach as quickly as possible.

MaxCloutier · 2023-08-23T16:58:14Z

we need this functionality in components where the ActionList is not controlled by consumers of Polaris

I see now. Yeah, that limitation is certainly a blocker to the Listbox strategy.

Thank you for your patience as I've continued to build my understanding of the limitations and constraints, @MaxCloutier! Let's get the connectedDisclosure PR merged and try the component re-use approach.

Sorry for blocking this. I know this is impacting our merchant UX and want to resolve a smart systematic approach as quickly as possible.

No worries at all @samrose3! I do want to merge this, but want to do it in a way that makes sense for your team and makes sure we have a decent solution on hand :)

sam-b-rose · 2023-08-24T13:50:05Z

@MaxCloutier would you have the bandwidth to merge this but then open a new PR targeting next that removes the duplicate component code? That way we can get this feature in now but trim it up in the next major.

MaxCloutier · 2023-08-24T14:00:38Z

@MaxCloutier would you have the bandwidth to merge this but then open a new PR targeting next that removes the duplicate component code? That way we can get this feature in now but trim it up in the next major.

Works for me! I'll need @kyledurand's approval though since he requested changes

* main: Fix rename prop migration (#10215) Update translations: default (#10219) [Avatar] Override SE23 background override with imageHasLoaded (#10211) Create Code style guide.md (#10176) ActionsList filtering (#9907) Version Packages (#10078) Support dynamic fontsizes in iOS devices (#10181) Fix migration resolution path (#10188) [Button] Deprecate connectedDisclosure (#10183) Update sandbox.md (#10186) Allow migrator to target subcomponents and change prop values (#10071) Remove `polaris-cli` and `polaris-codemods` packages (#10101) Bump loader-utils from 1.4.0 to 1.4.2 (#7725) Bump deep-object-diff from 1.1.7 to 1.1.9 (#7734) Bump decode-uri-component from 0.2.0 to 0.2.2 (#7832) Bump webpack from 5.72.0 to 5.76.0 (#8667) Bump fastify from 4.5.3 to 4.15.0 (#8750) [Modal] Activator no longer wrapped in Box (#10086) Fix icon optimize config (#10087) [ResourceList] Remove layout jank when bulk actions enabled and item selected

@kyledurand

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @shopify/polaris-migrator@0.21.0 ### Minor Changes - [#10246](#10246) [`1947b4691`](1947b46) Thanks [@kyledurand](https://github.com/kyledurand)! - Added the ability to migrate prop values ### Patch Changes - [#10215](#10215) [`e4a6d47ab`](e4a6d47) Thanks [@aveline](https://github.com/aveline)! - Fixed a bug in the rename prop migration ## @shopify/polaris@11.14.0 ### Minor Changes - [#9907](#9907) [`ef7ddb4ac`](ef7ddb4) Thanks [@MaxCloutier](https://github.com/MaxCloutier)! - Add a search field to filter ActionList that have more than 10 items ### Patch Changes - [#10288](#10288) [`cd1578230`](cd15782) Thanks [@ssetem](https://github.com/ssetem)! - Only apply apple dynamic text to mobile breakpoint - [#10292](#10292) [`72f55e32f`](72f55e3) Thanks [@kyledurand](https://github.com/kyledurand)! - Added check for string type before calling string method - [#10211](#10211) [`ac044b3c9`](ac044b3) Thanks [@peterlazzarino](https://github.com/peterlazzarino)! - Ensure Avatar has no background color if an source prop is passed in to allow for transparent images - [#10287](#10287) [`9b14e231a`](9b14e23) Thanks [@laurkim](https://github.com/laurkim)! - Fixed focus ring and input text styles on `ActionList.SearchField` component ## polaris.shopify.com@0.57.2 ### Patch Changes - Updated dependencies \[[`ef7ddb4ac`](ef7ddb4), [`cd1578230`](cd15782), [`72f55e32f`](72f55e3), [`ac044b3c9`](ac044b3), [`9b14e231a`](9b14e23)]: - @shopify/polaris@11.14.0 Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

henryyi · 2023-09-07T20:55:33Z

This change has inadvertently added the search field to the admin user menu:

We should either add the ability to opt-out or make this opt-in only.

MaxCloutier · 2023-09-08T16:26:31Z

This change has inadvertently added the search field to the admin user menu:

We should either add the ability to opt-out or make this opt-in only.

@henryyi will work on making it opt-in

henryyi · 2023-09-08T16:31:57Z

Thanks @MaxCloutier we have an open issue if you want to coordinate tracking: Shopify/workflows-merchant-guidance/issues/660

MaxCloutier · 2023-09-15T16:52:53Z

@henryyi This has just been merged in Polaris and it will be opt-in only from now on.

Closes Shopify#8686 ## Why? With extension GA comes a lot of potential new items in the `More Actions` of pages supporting extensibility making it increasingly more difficult for merchants to find what they are looking for. The median number of links in "More action" menus on the products, orders, and customers pages for high GMV merchants is already between 12 and 31. This number does not count any non-app actions that have been added, so in reality, the number of actions is even higher. This is a terrible UX for our merchants. ## Proposed Changes Adding a search input that will appear anytime an ActionList has more than 10 elements. <img width="1015" alt="Screenshot 2023-08-09 at 9 06 13 AM" src="https://github.com/Shopify/ui-api-design/assets/3555826/cb021493-ada4-4ac2-8991-42a6d979207a"> <img width="1261" alt="Screenshot 2023-08-10 at 1 52 48 PM" src="https://github.com/Shopify/polaris/assets/3555826/6c2d9763-6b81-4808-9659-eb1db7b4bf1c"> This will give us the filtering behaviour in various areas and cases for free: - When actionGroups are rolledUp - In Bulk Actions - When actionGroups are not rolledUp, but still have more than 10 items This will also enable the rest of the admin to leverage this behaviour for free to give a consistent merchant experience. ## Open questions Using the TextField (or SearchField from filters) creates a circular dependency with the ActionList. What would be the right approach to have the search input with an icon without creating circular dependencies? ## Resources - Action Extensions tech doc - https://docs.google.com/document/d/1BEAHpkfeyUTaxW3B18LjojimDmqNLr1Fc2wfpMXzQ64/edit - Mockups for Action Extensions - https://www.figma.com/proto/afrUykUTB0n4GB4evE5F6R/%F0%9F%92%8EAdmin-UI-extension-patterns?node-id=398-111475&scaling=scale-down&page-id=295%3A118673&starting-point-node-id=398%3A111475&show-proto-sidebar=1 - Post in Admin UX (WxM) with an initial proposal - https://shopify.workplace.com/groups/1294395484626415/permalink/1321299791935984/ - Investigation of changes needed - Shopify/app-ui#36 (comment) --------- Co-authored-by: translation-platform[bot] <34770790+translation-platform[bot]@users.noreply.github.com>

MaxCloutier added the 🤖Skip Comment Check Skip the migrator comment CI check label Aug 11, 2023

MaxCloutier marked this pull request as ready for review August 11, 2023 14:17

translation-platform bot reviewed Aug 11, 2023

View reviewed changes

Fionoble approved these changes Aug 11, 2023

View reviewed changes

MaxCloutier requested a review from a team August 11, 2023 14:39

elanalynn approved these changes Aug 11, 2023

View reviewed changes

sam-b-rose reviewed Aug 15, 2023

View reviewed changes

MaxCloutier added 12 commits August 16, 2023 09:55

add search box on action groups

e0d813a

add search box on action groups on mobile

cc5a435

Update implementation

c4ee485

Cleanup

3353f29

Cleanup

42ab0e6

Cleanup cleanup

0ada929

implement search field

50756c0

fix tests

c326e8f

Add tests

f4461ec

cleanup

0ec5cda

changeset

879494e

Fix placholder

bcedf45

MaxCloutier force-pushed the max/page-action-groups-searchbar branch from c9bc6d4 to bcedf45 Compare August 16, 2023 13:55

MaxCloutier added 2 commits August 16, 2023 09:56

Adjust amount of items needed for filter

5f10916

Fix tests

deb201d

Update 6 translation files

29a773d

kyledurand requested changes Aug 23, 2023

View reviewed changes

kyledurand approved these changes Aug 24, 2023

View reviewed changes

MaxCloutier merged commit ef7ddb4 into main Aug 24, 2023
14 checks passed

MaxCloutier deleted the max/page-action-groups-searchbar branch August 24, 2023 14:08

shopify-github-actions-access bot mentioned this pull request Aug 24, 2023

Version Packages #10204

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ActionsList filtering #9907

ActionsList filtering #9907

MaxCloutier commented Aug 9, 2023 •

edited

Loading

translation-platform bot Aug 11, 2023

translation-platform bot Aug 11, 2023

translation-platform bot Aug 11, 2023

elanalynn left a comment

elanalynn Aug 11, 2023

MaxCloutier Aug 14, 2023

sam-b-rose Aug 15, 2023

sam-b-rose Aug 15, 2023

MaxCloutier Aug 15, 2023

sam-b-rose commented Aug 16, 2023

MaxCloutier commented Aug 18, 2023

sam-b-rose commented Aug 18, 2023 •

edited

Loading

MaxCloutier commented Aug 22, 2023

sam-b-rose commented Aug 23, 2023

MaxCloutier commented Aug 23, 2023

kyledurand left a comment

MaxCloutier commented Aug 23, 2023

sam-b-rose commented Aug 23, 2023

MaxCloutier commented Aug 23, 2023

sam-b-rose commented Aug 24, 2023

MaxCloutier commented Aug 24, 2023

henryyi commented Sep 7, 2023

MaxCloutier commented Sep 8, 2023

henryyi commented Sep 8, 2023

MaxCloutier commented Sep 15, 2023

ActionsList filtering #9907

ActionsList filtering #9907

Conversation

MaxCloutier commented Aug 9, 2023 • edited Loading

Why?

Proposed Changes

Open questions

Resources

translation-platform bot Aug 11, 2023

Choose a reason for hiding this comment

translation-platform bot Aug 11, 2023

Choose a reason for hiding this comment

translation-platform bot Aug 11, 2023

Choose a reason for hiding this comment

elanalynn left a comment

Choose a reason for hiding this comment

elanalynn Aug 11, 2023

Choose a reason for hiding this comment

MaxCloutier Aug 14, 2023

Choose a reason for hiding this comment

sam-b-rose Aug 15, 2023

Choose a reason for hiding this comment

sam-b-rose Aug 15, 2023

Choose a reason for hiding this comment

MaxCloutier Aug 15, 2023

Choose a reason for hiding this comment

sam-b-rose commented Aug 16, 2023

MaxCloutier commented Aug 18, 2023

sam-b-rose commented Aug 18, 2023 • edited Loading

Proposal

MaxCloutier commented Aug 22, 2023

sam-b-rose commented Aug 23, 2023

MaxCloutier commented Aug 23, 2023

kyledurand left a comment

Choose a reason for hiding this comment

MaxCloutier commented Aug 23, 2023

sam-b-rose commented Aug 23, 2023

MaxCloutier commented Aug 23, 2023

sam-b-rose commented Aug 24, 2023

MaxCloutier commented Aug 24, 2023

henryyi commented Sep 7, 2023

MaxCloutier commented Sep 8, 2023

henryyi commented Sep 8, 2023

MaxCloutier commented Sep 15, 2023

MaxCloutier commented Aug 9, 2023 •

edited

Loading

sam-b-rose commented Aug 18, 2023 •

edited

Loading