feat(table): update sortable demo to align with core #7292

jenny-s51 · 2022-04-22T19:07:29Z

What: Closes #7272

Additional issues:

patternfly-build · 2022-04-22T19:15:50Z

Preview: https://patternfly-react-pr-7292.surge.sh

A11y report: https://patternfly-react-pr-7292-a11y.surge.sh

mmenestr · 2022-04-27T19:41:01Z

Is this demo the one I'm supposed to be checking for this PR? https://patternfly-react-pr-7292.surge.sh/components/table#composable-sortable---custom-control

mcoker · 2022-04-27T20:16:21Z

@mmenestr it's this one

https://patternfly-react-pr-7292.surge.sh/components/table/react-demos/sortable---responsive/

wise-king-sullyman

Is this the right core demo to be looking at? https://www.patternfly.org/v4/components/table/html-demos/sortable

If so I'm not sure if these differences are substantive or not, but I notice a handful of differences between the demos still.

Areas of difference:

Masthead logo
Masthead toolbar items
Breadcrumbs below the masthead
Items in the toolbar group
Checkboxes in the rows
Pagination at the bottom of the table

wise-king-sullyman · 2022-05-06T20:40:37Z

I'm seeing a ReferenceError: number is not defined error when navigating to the demo now.

wise-king-sullyman · 2022-05-06T22:02:35Z

I'm still seeing a handful of differences in the toolbars, specifically at larger window widths. I.e.

vs

The title and description are also different, but I think that the title and description in this demo should be copied into the core demo if anything.

I'm not sure if those are significant since this demo seems to focus on small window usage.

jenny-s51 · 2022-05-09T20:30:02Z

Thank you for your feedback @wise-king-sullyman ! I've added some changes that should make the toolbar and dashboard header align with the core demos more closely.

Per @tlabaj and @mcarrano, we are separating the implementation of additional toolbar logic from these table PRs. We will address that in an upcoming sprint as these must be resolved first:

Please kindly re-review when you have a moment so we can prioritize getting these table changes in for the release 🙂

wise-king-sullyman

If the additional toolbar differences will be resolved in future issues this looks perfect 🔥

tlabaj

Looks pretty good. Only thing I am seeing is the the "Responsive action hidden on small button" is not getting hidden on small for me.

tlabaj

@jenny-s51 this still is not working correctly. I am also noticing that the structure does not match core. Core's toolbar seems to be in a card and this one is not. I am also seeing other discrepancies lie that toolbar content section in core has "pf-m-nowrap" applied.

cc @mcoker @mattnolting

tlabaj

It is looking better! however I see the "sort" button disappearing when the screen is very small (phone size)

Here it shows when it is bigger (tablet size).

mcarrano · 2022-05-10T21:01:30Z

Good catch @tlabaj

@jenny-s51 for reference, this core demo shows how sorting should behave when the view port shrinks: https://www.patternfly.org/v4/components/table/html-demos/sortable/ Basically a sort menu is exposed in the toolbar after the table converts to its stacked orientation.

jenny-s51 · 2022-05-11T18:29:41Z

@tlabaj @mcarrano I've updated with a fix that keeps the sort menu in the toolbar when using a phone size viewport.

mcarrano

This looks great. Thanks @jenny-s51 !

tlabaj

The responsive behaviour is still slightly off. I see the sorting button before the table transitions to a stacked layout.

mcoker

This LGTM, left a few comments but none are blockers.

packages/react-core/src/demos/examples/DashboardHeader.js

packages/react-table/src/demos/Table.md

tlabaj · 2022-05-16T19:13:07Z

I agree @jenny-s51 updating the wrapper is outside the scope of this PR. I have created this issue #7429

mcoker

Looks like the sort icon is hidden on xs (< sm breakpoint) viewports - I think this line can just be updated to {{ md: 'hidden' }}?

https://github.com/patternfly/patternfly-react/blame/1e7104949ff012ac1246d2eb65250131458b653e/packages/react-table/src/demos/Table.md#L2282

mcoker

LGTM! Great job on this!!

tlabaj

Looks great Jenny!

patternfly-build · 2022-05-19T17:57:10Z

Your changes have been released in:

eslint-plugin-patternfly-react@4.55.0
@patternfly/react-catalog-view-extension@4.67.0
@patternfly/react-charts@6.69.0
@patternfly/react-code-editor@4.57.0
@patternfly/react-console@4.67.0
@patternfly/react-core@4.216.0
@patternfly/react-docs@5.77.0
@patternfly/react-icons@4.67.0
@patternfly/react-inline-edit-extension@4.61.0
demo-app-ts@4.176.0
@patternfly/react-integration@4.178.0
@patternfly/react-log-viewer@4.61.0
@patternfly/react-styles@4.66.0
@patternfly/react-table@4.85.0
@patternfly/react-tokens@4.68.0
@patternfly/react-topology@4.63.0
@patternfly/react-virtualized-extension@4.63.0
transformer-cjs-imports@4.54.0

Thanks for your contribution! 🎉

jenny-s51 changed the title ~~Sortable table fullscreen~~ feat(table): update sortable demo to align with core Apr 22, 2022

tlabaj added css review labels Apr 25, 2022

tlabaj requested review from kmcfaul, mcoker and mmenestr April 25, 2022 18:55

wise-king-sullyman reviewed May 5, 2022

View reviewed changes

wise-king-sullyman approved these changes May 9, 2022

View reviewed changes

tlabaj reviewed May 9, 2022

View reviewed changes

jenny-s51 force-pushed the sortableTableFullscreen branch from c335b22 to 03e34f9 Compare May 9, 2022 21:41

gitdallas approved these changes May 9, 2022

View reviewed changes

kmcfaul approved these changes May 10, 2022

View reviewed changes

tlabaj requested changes May 10, 2022

View reviewed changes

jenny-s51 force-pushed the sortableTableFullscreen branch from 8639062 to b332158 Compare May 10, 2022 17:15

tlabaj requested changes May 10, 2022

View reviewed changes

jenny-s51 requested a review from tlabaj May 11, 2022 18:52

mcarrano approved these changes May 11, 2022

View reviewed changes

jenny-s51 added ux approved and removed ux review labels May 11, 2022

tlabaj requested changes May 11, 2022

View reviewed changes

mcoker approved these changes May 11, 2022

View reviewed changes

packages/react-core/src/demos/examples/DashboardHeader.js Outdated Show resolved Hide resolved

packages/react-table/src/demos/Table.md Outdated Show resolved Hide resolved

packages/react-table/src/demos/Table.md Show resolved Hide resolved

jenny-s51 force-pushed the sortableTableFullscreen branch from becebbb to 30d6637 Compare May 12, 2022 20:45

jenny-s51 requested a review from tlabaj May 13, 2022 17:36

jenny-s51 mentioned this pull request May 16, 2022

feat(table): Add fullscreen demo for compact variant #7338

Merged

jenny-s51 added css approved and removed css review labels May 16, 2022

mmenestr approved these changes May 16, 2022

View reviewed changes

mcoker requested changes May 17, 2022

View reviewed changes

jenny-s51 force-pushed the sortableTableFullscreen branch from 1e71049 to e1ca0ec Compare May 17, 2022 18:48

jenny-s51 added 15 commits May 17, 2022 15:12

updates to table

4bdcf66

sortable

dc828c1

revert divider changes

299a318

update paths from master

20f0e8e

add select logic, pagination, dashboard wrapper

a36eab3

fix type error

5e88040

align dashboard header with core

e59f50b

update toolbar to more closely match core, remove unused imports

f062799

PR feedback from Titani

4aa7de8

toolbar and page updates

74a0ef3

PR feedback from Titani

72d78bd

display sorting menu once table switches to stacked layout

90f11dd

PR feedback from Michael

b0b32d3

add app launcher dropdown and remove actions from aria labels

67c5665

fix filter icon visibility

5668a56

jenny-s51 force-pushed the sortableTableFullscreen branch from 5b5dfcd to 5668a56 Compare May 17, 2022 19:21

divider files

33952c4

jenny-s51 requested a review from mcoker May 18, 2022 14:52

mcoker approved these changes May 19, 2022

View reviewed changes

tlabaj approved these changes May 19, 2022

View reviewed changes

tlabaj merged commit 6891764 into patternfly:main May 19, 2022

feat(table): update sortable demo to align with core #7292

feat(table): update sortable demo to align with core #7292

Uh oh!

Conversation

jenny-s51 commented Apr 22, 2022

Uh oh!

patternfly-build commented Apr 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmenestr commented Apr 27, 2022

Uh oh!

mcoker commented Apr 27, 2022

Uh oh!

wise-king-sullyman left a comment

Choose a reason for hiding this comment

Uh oh!

wise-king-sullyman commented May 6, 2022

Uh oh!

wise-king-sullyman commented May 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jenny-s51 commented May 9, 2022

Uh oh!

wise-king-sullyman left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcarrano commented May 10, 2022

Uh oh!

jenny-s51 commented May 11, 2022

Uh oh!

mcarrano left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlabaj commented May 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

tlabaj left a comment

Choose a reason for hiding this comment

Uh oh!

patternfly-build commented May 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

patternfly-build commented Apr 22, 2022 •

edited

Loading

wise-king-sullyman commented May 6, 2022 •

edited

Loading

tlabaj left a comment •

edited

Loading

tlabaj left a comment •

edited

Loading

tlabaj commented May 16, 2022 •

edited

Loading