Skip to content

feat(table): add compound expandable fullscreen demo#7366

Merged
tlabaj merged 10 commits into
patternfly:mainfrom
jenny-s51:iss7339
May 18, 2022
Merged

feat(table): add compound expandable fullscreen demo#7366
tlabaj merged 10 commits into
patternfly:mainfrom
jenny-s51:iss7339

Conversation

@jenny-s51

@jenny-s51 jenny-s51 commented May 5, 2022

Copy link
Copy Markdown
Contributor

@patternfly-build

patternfly-build commented May 5, 2022

Copy link
Copy Markdown
Collaborator

@nicolethoen nicolethoen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is the sortable columns in the core example something we should add to the react example? I notice the compound expandable example (not the demo) does not have sortable columns, so I'm not sure how hard that'd be to wire up...
What do you think, @mcarrano ?

@mcarrano

mcarrano commented May 6, 2022

Copy link
Copy Markdown
Member

It seems like sortable columns and compound expansion and really demonstrating two separate things. So I'm OK to leave it out of this demo unless there is value in including it here because there are unique issues to integrating column sorting with compound expansion that do not occur elsewhere.

@mmenestr

mmenestr commented May 6, 2022

Copy link
Copy Markdown

Something random I noticed is that clicking on the "Open in GitHub" link on the first row, opens up the second column expansion. That probably shouldn't happen right?

@jenny-s51

jenny-s51 commented May 6, 2022

Copy link
Copy Markdown
Contributor Author

It seems like sortable columns and compound expansion and really demonstrating two separate things. So I'm OK to leave it out of this demo unless there is value in including it here because there are unique issues to integrating column sorting with compound expansion that do not occur elsewhere.

@mcarrano I agree that it makes the most sense to leave sorting out of this demo, since we are demonstrating compound expansion and don't want to puff up the demo too much with repeated code that doesn't directly relate to the table variant being demoed.

If consumers want to see an example of sorting, they can check out our sortable demo!.

Something random I noticed is that clicking on the "Open in GitHub" link on the first row, opens up the second column expansion. That probably shouldn't happen right?

Good question and nice catch @mmenestr ! For all our react demos with an action link, the href is set to #. When the link is clicked, the # tag is added to the url and the component is rerendered.

This effectively has the same function as refreshing the page, which is why the row gets expanded again (that's the default state here). This won't present any issues to consumers.

@tlabaj tlabaj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would make this a little more similar to core by

  1. Chang the name to compound expansion
  2. add a toolbar
  3. add bottom pagination

@mcarrano mcarrano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks @jenny-s51 !

@mcoker mcoker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@tlabaj tlabaj merged commit 2283b01 into patternfly:main May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat(Table): Full screen demo for compound expansion

7 participants