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

[PBNTR-331] Adding column sort to Advanced Table kit #3481

Merged
merged 4 commits into from
Jun 27, 2024

Conversation

carloslimasd
Copy link
Contributor

@carloslimasd carloslimasd commented Jun 24, 2024

What does this PR do?
Adding column sort to Advanced Table kit

Screenshots:
image

How to test? Steps to confirm the desired behavior:

  1. Go to the Advanced Table kit
  2. Scroll down to the Table Sort doc example

Checklist:

  • LABELS Add a label: enhancement, bug, improvement, new kit, deprecated, or breaking. See Changelog & Labels for details.
  • DEPLOY I have added the milano label to show I'm ready for a review.
  • TESTS I have added test coverage to my code.

@carloslimasd carloslimasd added enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano labels Jun 24, 2024
@carloslimasd carloslimasd self-assigned this Jun 24, 2024
@carloslimasd carloslimasd requested review from a team as code owners June 24, 2024 15:26
@powerhome-portal
Copy link

A change to documentation files was detected in your PR. Please visit this link to preview changes: https://portal-staging.powerapp.cloud/docs?filters[kind]=all&filters[user]=all&filters[namespaceFilter]=pbntr-331-advanced-table-sort

@nidaqg nidaqg self-requested a review June 25, 2024 18:49
Copy link
Contributor

@nidaqg nidaqg left a comment

Choose a reason for hiding this comment

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

Looks good! small thing @carloslimasd : when the page first loads this is the icon for sort that shows up:
Screenshot 2024-06-25 at 2 48 11 PM
which I believe is coming from the Table kit. Clicking it reloads the page, changes the icon but does NOT sort. Subsequent clicks sort as expected

@carloslimasd
Copy link
Contributor Author

Looks good! small thing @carloslimasd : when the page first loads this is the icon for sort that shows up: Screenshot 2024-06-25 at 2 48 11 PM which I believe is coming from the Table kit. Clicking it reloads the page, changes the icon but does NOT sort. Subsequent clicks sort as expected

@nidaqg, thank you for the great feedback.
I talked to Colin about it, and the point is that for React, the lib is "pre-ordering" the list, and the default state is already an ordered list. On the Rails version, we are not ordering the list; this is the dev's responsibility, and we can not guarantee that the first data will be ordered. Since React and Rails are different, this icon will represent the first state (no ordered list).

@nidaqg nidaqg added Code Approved Approved by a Playbook Admin Product Approved pending technical review, OK to merge to master labels Jun 26, 2024
@jasperfurniss jasperfurniss added this pull request to the merge queue Jun 27, 2024
Merged via the queue into master with commit 580db4e Jun 27, 2024
7 checks passed
@jasperfurniss jasperfurniss deleted the PBNTR-331-advanced-table-sort branch June 27, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Approved Approved by a Playbook Admin enhancement New Features, Props, & Variants (USED IN CHANGELOG) milano 20 MAX - Deploy this PR to a review environment via Milano Product Approved pending technical review, OK to merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants