[IndexProvider/Table] Migrate from web - take 2 #3646

AndrewMusgrave · 2020-11-18T20:26:38Z

WHY are these changes introduced?

Feature requests: [#390, #3637]

As teams reach AODA they'll need to replace their resource list with headers, with a semantic table. This pull request is bringing the table from orders into Polaris in an unstable state so others can gain from its accessibility improvements and contribute to the stability of its API/name

Major hanges other than C+P

Add hook to streamline component usage

Add negative margin while in select mode with bulk actions to remove extra white space at the bottom of the list
Add tests
Change loading bar colors to meet contrast requirements

Notes

Accessibility tests are failing from the filter's component - going to address them in another pull request

WHAT is this pull request doing?

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      {/* Add the code you want to test in here */}
    </Page>
  );
}

Screenies

Simple index table

Simple small screen index table

Index table with empty state

Index table with bulk actions

Index table with bulk actions across pages

Index table with loading state

IndexTable with filtering

IndexTable with all of its elements

Small screen IndexTablr with all of its elements

github-actions · 2020-11-18T20:29:44Z

🟢 This pull request modifies 30 files and might impact 1 other files.

Details:

All files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

📄 .storybook/polaris-readme-loader.js (total: 0)

Files potentially affected (total: 0)

📄 locales/cs.json (total: 0)

Files potentially affected (total: 0)

📄 locales/da.json (total: 0)

Files potentially affected (total: 0)

📄 locales/de.json (total: 0)

Files potentially affected (total: 0)

📄 locales/en.json (total: 0)

Files potentially affected (total: 0)

📄 locales/es.json (total: 0)

Files potentially affected (total: 0)

📄 locales/fi.json (total: 0)

Files potentially affected (total: 0)

📄 locales/fr.json (total: 0)

Files potentially affected (total: 0)

📄 locales/it.json (total: 0)

Files potentially affected (total: 0)

📄 locales/ja.json (total: 0)

Files potentially affected (total: 0)

📄 locales/ko.json (total: 0)

Files potentially affected (total: 0)

📄 locales/nb.json (total: 0)

Files potentially affected (total: 0)

📄 locales/nl.json (total: 0)

Files potentially affected (total: 0)

📄 locales/pl.json (total: 0)

Files potentially affected (total: 0)

📄 locales/pt-BR.json (total: 0)

Files potentially affected (total: 0)

📄 locales/pt-PT.json (total: 0)

Files potentially affected (total: 0)

📄 locales/sv.json (total: 0)

Files potentially affected (total: 0)

📄 locales/th.json (total: 0)

Files potentially affected (total: 0)

📄 locales/tr.json (total: 0)

Files potentially affected (total: 0)

📄 locales/vi.json (total: 0)

Files potentially affected (total: 0)

📄 locales/zh-CN.json (total: 0)

Files potentially affected (total: 0)

📄 locales/zh-TW.json (total: 0)

Files potentially affected (total: 0)

🧩 src/components/IndexProvider/IndexProvider.tsx (total: 1)

Files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

🧩 src/components/IndexProvider/index.ts (total: 1)

Files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

🧩 src/components/IndexProvider/tests/IndexProvider.test.tsx (total: 0)

Files potentially affected (total: 0)

🎨 src/components/IndexTable/IndexTable.scss (total: 1)

Files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

🧩 src/components/IndexTable/IndexTable.tsx (total: 0)

Files potentially affected (total: 0)

📄 src/components/IndexTable/README.md (total: 0)

Files potentially affected (total: 0)

🧩 src/components/IndexTable/components/Cell/Cell.tsx (total: 1)

Files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

🧩 src/components/IndexTable/components/Cell/index.ts (total: 1)

Files potentially affected (total: 1)

/src/components/IndexTable/IndexTable.tsx

AndrewMusgrave · 2020-11-19T21:22:55Z

src/components/IndexTable/IndexTable.scss

@@ -72,7 +72,7 @@ $loading-panel-height: rem(53px);

 .LoadingPanelText {
  margin-left: rem(12px);
-  color: var(--p-text-highlight, color('teal', 'dark'));
+  color: var(--p-text, color('teal', 'darker'));


Had to adjust colors to meet contrast requirements. We'll want to take a look at making --p-text-highlight accessible.

AndrewMusgrave · 2020-11-19T21:23:30Z

Other accessibility tests are fixed in : #3651

AndrewMusgrave · 2020-11-20T16:19:30Z

.storybook/polaris-readme-loader.js

@@ -138,8 +138,8 @@ import {
  Heading,
  Icon,
  Image,
-  IndexProvider,
-  IndexTable,
+  UnstableIndexProvider as IndexProvider,


Commit adding unstable prefix

I was thinking about this being marked "unstable" again. What would it take for us to consider this stable and be confident in this being a v1 of the component? We have been running this in production now for quite some time, it seems like from a stability perspective we should be confident at this point.

The instability is regarding the API since we've never revisited it. I was hoping to take a look at it this week but didn't have the chance. I'm going on vacation but ill take a look when I'm back and see if we can resolve the API

AndrewMusgrave · 2020-11-20T16:19:59Z

.storybook/polaris-readme-loader.js

@@ -194,7 +194,7 @@ import {
  UnstyledLink,
  VisuallyHidden,
  VideoThumbnail,
-  useIndexResourceState,
+  useIndexResourceStateUnstable as useIndexResourceState,


Used suffix for the hook - since hooks always start with use as per convention

translation-platform · 2020-11-20T16:39:33Z

locales/en.json

@@ -117,6 +117,28 @@
    "Icon": {
      "backdropWarning": "The {color} icon doesn’t accept backdrops. The icon colors that have backdrops are: {colorsWithBackDrops}"
    },
+    "IndexProvider": {
+      "defaultItemSingular": "Item",


Poorly localized source string detected

List of problems detected:

Key Polaris.IndexProvider.defaultItemSingular contains the word singular. While this in itself is not an issue, it is a good indicator that you should be making use of the pluralization subkeys one and other instead (see String Externalization Guide).

Questions about these messages? Hop in the #help-i18n-and-translation Slack channel.

translation-platform · 2020-11-20T16:39:34Z

locales/en.json

@@ -117,6 +117,28 @@
    "Icon": {
      "backdropWarning": "The {color} icon doesn’t accept backdrops. The icon colors that have backdrops are: {colorsWithBackDrops}"
    },
+    "IndexProvider": {
+      "defaultItemSingular": "Item",
+      "defaultItemPlural": "Items",


Poorly localized source string detected

List of problems detected:

Key Polaris.IndexProvider.defaultItemPlural contains the word plural. While this in itself is not an issue, it is a good indicator that you should be making use of the pluralization subkeys one and other instead (see String Externalization Guide).

Questions about these messages? Hop in the #help-i18n-and-translation Slack channel.

dfmcphee · 2020-12-01T14:59:47Z

.storybook/polaris-readme-loader.js

@@ -138,8 +138,8 @@ import {
  Heading,
  Icon,
  Image,
-  IndexProvider,
-  IndexTable,
+  UnstableIndexProvider as IndexProvider,


I was thinking about this being marked "unstable" again. What would it take for us to consider this stable and be confident in this being a v1 of the component? We have been running this in production now for quite some time, it seems like from a stability perspective we should be confident at this point.

src/components/IndexProvider/IndexProvider.tsx

src/components/IndexTable/IndexTable.tsx

src/components/IndexTable/README.md

dfmcphee · 2020-12-01T15:09:42Z

src/components/IndexTable/README.md

+
+### Simple small screen index table
+
+A small screen index table with simple items and no bulk actions, sorting, or filtering.


I wonder if rather than having a separate example like this that looks strange when viewed on larger screens, we instead have one example that actually does the responsive switch between the views. I think that would be more helpful to consumers who are trying to wire this up.

@umar-k Is adding responsive examples here 🎉

Alexzanderk · 2021-01-10T19:24:22Z

@alex-page @dfmcphee @translation-platform Hi guys! How is it going with this PR?

AndrewMusgrave · 2021-01-11T14:47:39Z

tests/setup.ts

@@ -7,6 +7,13 @@ configure({adapter: new Adapter()});
 // Mocks for scrolling
 window.scroll = () => {};

+window.open = (


Adding a mock for window.open

AndrewMusgrave · 2021-01-11T14:48:09Z

src/components/IndexTable/tests/IndexTable.test.tsx

  );
 }

 const mockRenderRow = (item: any) => {
  return <Component {...item} key={item.id} />;
 };

+const mockRenderCondensedRow = (item: any) => {


Adding a second renderer to remove the console errors

AndrewMusgrave · 2021-01-11T14:48:40Z

src/components/IndexTable/components/Row/tests/Row.test.tsx

@@ -41,13 +41,9 @@ describe('<Row />', () => {

  it('renders a RowHoveredContext provider', () => {
    const row = mountWithTable(
-      <table>


We're already using mountWithTable

AndrewMusgrave · 2021-01-11T15:01:40Z

@dfmcphee I took a look at Checkbox, Row & Cell.

Row didn't need any changes.

For Cell & Checkbox, I removed the last & sub props since they're not used in production. I changed noPadding to flush to match Modal's API. first has also been removed. The styles are being applied through a selector rather than a second class. Row now renders a checkbox by default, allowing us to remove IndexTable.Checkbox from the API assisting in small screen view creation.

I briefly looked at IndexTable & IndexProvider. I removed onMoreActionPopoverToggle for now till we're more confident about it. On Friday I'm hoping to finish reworking these. I want to take a look changing the sort prop on IndexTable and see if we have a better way of handling it. We can also remove resolveItemId from 'IndexProvider` & I'm thinking we'll be able to combine the components since we don't get a lot of value from having them separate.

What do you think?

dfmcphee · 2021-01-13T14:52:12Z

That all sounds great to me. I think combining makes sense, at least for consumers right now. It still would be nice to be able to have some of the index logic abstracted from the view for other types in the future (index grid for example). Could we keep the provider but have it automatically included in the table?

lsit · 2021-01-20T18:22:22Z

Hi! Love this PR! We (Merchandising) have converted Products and Collections to use the IndexTable as well and we've collected some improvements here. If you need help testing or reviewing, I'm sure @xie or I could help out. Thanks for doing this! 🎉

GhostApps · 2021-01-26T20:12:32Z

Is this going to be in v6?

AndrewMusgrave · 2021-01-26T20:48:20Z

Hi @GhostApps, this will not be bundled with v6. V6 primarily revolves around design language and accessibility standards (AODA). This change would possibly delay the v6 rollout. Ideally, we'll have this ready shortly after and ship it out in a minor version!

umar-k

Component looks really good 👍

I noticed one issue with the horizontal scrollbar during 🎩
It looks like the scrollBarContentStyles doesn't get an updated width when the window is resized. It causes a scrollbar even when it's not needed.

indexTableScrollBar.mov

umar-k · 2021-02-05T16:07:34Z

src/components/IndexTable/IndexTable.tsx

+    tableElement.current && tableInitialized
+      ? {
+          height: '1px',
+          width: tableElement.current.offsetWidth - SCROLL_BAR_PADDING,


Is tableElement.current.offsetWidth supposed to change with every render when the browser is being resized?
It seems to stay the same and might be whats causing the scrollbar issue.

@lhoffbeck Sounds like this might be what you noticed this morning

Yeah that looks exactly like what I was seeing!

@umar-k Should be fixed now - here's the commit

AndrewMusgrave · 2021-02-05T17:40:26Z

We still need 3 required languages to 🚢 however the estimate is that they'll come in over the weekend.

umar-k

🎩 'd and looks good to me! 🎉

GhostApps · 2021-02-09T19:21:11Z

@AndrewMusgrave any chance you know when this will be released? I tried to play with it by pulling latest but couldn't get the playground or building for a consuming project to work so am having to wait for the next official Polaris release

AndrewMusgrave · 2021-02-09T21:03:49Z

@GhostApps The Polaris team will have more context, but they usually draft a release ~2 weeks. The last one was 11 days ago so it shouldn't be to long until a release

What error were you getting in the playground? Maybe we could sort that out.

AndrewMusgrave · 2021-02-09T21:06:10Z

If you wanted to create an issue and ping me there I would have happy to answer (so everyone in this pull request doesn't get pinged 😄 )

* Initial migration * Fix differences between web & polaris * Remove local translations * Fix styles & add useIndexResourceSelection * Added more tests * Docs & fixes & polish * Fix errors * Improve loading constrast * Add unstable prefix * Rework Checkbox, Row & Cell API * Remove onMoreActionPopoverToggle * Change imports to remove circular deps & delete subrow * Convert indexprovider internals to hooks * Removed index provider export * Update docs to exclude indextable * Apply feedback * Fix errors & tighten types * Rebase with main & remove NDL * Add new known custom property * Add useIndexResourceSelection tests * Add more tests * More tests * Add tests and remove unused code * Finish adding test for all reachable code * Update locales/fi.json * Update locales/fr.json * Update locales/zh-TW.json * Update locales/pt-PT.json * Update locales/th.json * Update locales/ja.json * Update locales/da.json * Update locales/ko.json * Update locales/pl.json * Update locales/it.json * Update locales/zh-CN.json * Update locales/sv.json * Update locales/nb.json * Update locales/nl.json * Fix scrollbar issue * Add known custom property * Update locales/vi.json * Update locales/es.json * Update locales/pt-BR.json * Update locales/tr.json * Update locales/de.json * Update locales/cs.json Co-authored-by: translation-platform <35075727+translation-platform@users.noreply.github.com>

AndrewMusgrave force-pushed the add-unstable-view branch from 77811e6 to 99fc4b0 Compare November 18, 2020 21:40

AndrewMusgrave mentioned this pull request Nov 19, 2020

[Filters] Removed id from proxy buttons #3651

Merged

AndrewMusgrave force-pushed the add-unstable-view branch from 8146afe to 95f685b Compare November 19, 2020 21:22

AndrewMusgrave commented Nov 19, 2020

View reviewed changes

AndrewMusgrave added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Nov 20, 2020

AndrewMusgrave commented Nov 20, 2020

View reviewed changes

AndrewMusgrave changed the title ~~[WIP][Index] Migrate #2~~ [IndexProvider/Table] Migrate from web - take 2 Nov 20, 2020

AndrewMusgrave marked this pull request as ready for review November 20, 2020 16:38

AndrewMusgrave requested review from dfmcphee and alex-page November 20, 2020 16:39

translation-platform bot reviewed Nov 20, 2020

View reviewed changes

dfmcphee reviewed Dec 1, 2020

View reviewed changes

alex-page force-pushed the master branch from ee1f920 to 405548f Compare December 21, 2020 18:28

GhostApps mentioned this pull request Jan 10, 2021

Added unstable index table #3097

Closed

AndrewMusgrave commented Jan 11, 2021

View reviewed changes

alex-page force-pushed the master branch from 37b047c to 9228d6e Compare January 12, 2021 21:40

alex-page force-pushed the master branch from 4eab9e5 to 9e09ba7 Compare January 15, 2021 05:17

Base automatically changed from master to main January 21, 2021 15:38

translation-platform added 10 commits February 5, 2021 10:44

Update locales/th.json

721a81f

Update locales/ja.json

2470f5c

Update locales/da.json

5d5fbc4

Update locales/ko.json

18d27f3

Update locales/pl.json

652a755

Update locales/it.json

a51c8c2

Update locales/zh-CN.json

520a0e6

Update locales/sv.json

aba4917

Update locales/nb.json

c5c8871

Update locales/nl.json

1c0f824

umar-k reviewed Feb 5, 2021

View reviewed changes

Fix scrollbar issue

b529ce3

AndrewMusgrave and others added 7 commits February 5, 2021 12:45

Add known custom property

3748f19

Update locales/vi.json

9a40a2a

Update locales/es.json

fb53c7e

Update locales/pt-BR.json

ef22385

Update locales/tr.json

1e8d04e

Update locales/de.json

f188e2d

Update locales/cs.json

1f39a1c

umar-k approved these changes Feb 5, 2021

View reviewed changes

AndrewMusgrave merged commit a040dea into main Feb 5, 2021

AndrewMusgrave deleted the add-unstable-view branch February 5, 2021 20:30

umar-k mentioned this pull request Feb 25, 2021

Update CHANGELOG.md for 6.1.0 #4022

Merged

This was referenced Mar 18, 2021

Bump @shopify/polaris from 6.0.1 to 6.2.0 jonpulsifer/hub#81

Closed

Bump @shopify/polaris from 5.12.0 to 6.2.0 jonpulsifer/pulsifer.ca#101

Closed


		### Simple small screen index table

		A small screen index table with simple items and no bulk actions, sorting, or filtering.

[IndexProvider/Table] Migrate from web - take 2 #3646

[IndexProvider/Table] Migrate from web - take 2 #3646

Conversation

AndrewMusgrave commented Nov 18, 2020 • edited Loading

WHY are these changes introduced?

Major hanges other than C+P

Notes

WHAT is this pull request doing?

How to 🎩

Screenies

Simple index table

Simple small screen index table

Index table with empty state

Index table with bulk actions

Index table with bulk actions across pages

Index table with loading state

IndexTable with filtering

IndexTable with all of its elements

Small screen IndexTablr with all of its elements

github-actions bot commented Nov 18, 2020 • edited Loading

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 1)

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 1)

Files potentially affected (total: 1)

Choose a reason for hiding this comment

AndrewMusgrave commented Nov 19, 2020

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

translation-platform bot Nov 20, 2020

Choose a reason for hiding this comment

translation-platform bot Nov 20, 2020

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

Alexzanderk commented Jan 10, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrewMusgrave commented Jan 11, 2021

dfmcphee commented Jan 13, 2021

lsit commented Jan 20, 2021

GhostApps commented Jan 26, 2021

AndrewMusgrave commented Jan 26, 2021

umar-k 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

AndrewMusgrave commented Feb 5, 2021

umar-k left a comment

AndrewMusgrave commented Nov 18, 2020 •

edited

Loading

github-actions bot commented Nov 18, 2020 •

edited

Loading

AndrewMusgrave commented Feb 9, 2021 •

edited

Loading