Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

[Terra-Paginator] Added hidePageCount prop to hide default page count provided by Paginator #3924

Merged
merged 6 commits into from
Sep 30, 2023

Conversation

supreethmr
Copy link
Contributor

@supreethmr supreethmr commented Sep 22, 2023

Summary

Added hidePageCount prop to hide default page count provided by Paginator. so that user can set custom page count.

What was changed:
Progressive Paginator and Controlled Progressive Paginator has been updated to hide the default page count ( highlighted on below screenshot )

Screenshot 2023-09-22 at 10 26 53 PM

Why it was changed:
To support passivity for fusion pass through of paginator and To allow user to pass in the count in desired format along with customPageLabel when required.

Testing

This change was tested using:

  • WDIO
  • Jest
  • Visual testing (please attach a screenshot or recording)
  • Other (please describe below)
  • No tests are needed

Updated terra-core --> tests ---> Paginator ---> Progressive Paginator With Custom Label to hide default Page count when hidePageCount is set to true

Reviews

In addition to engineering reviews, this PR needs:

  • UX review
  • Accessibility review
  • Functional review

Additional Details

This PR resolves:

UXPLATFORM-9662


Thank you for contributing to Terra.
@cerner/terra

@supreethmr supreethmr requested a review from a team as a code owner September 22, 2023 17:04
@supreethmr supreethmr self-assigned this Sep 22, 2023
Copy link
Contributor

@chenyixuan625 chenyixuan625 left a comment

Choose a reason for hiding this comment

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

One thing that I want to call out is that, with this change, when the consumer sets a static pageLabel string on the ProgressivePaginator https://github.com/cerner/terra-core/pull/3924/files#diff-5e4d0230ddc94d1702b4ce988ef0f63246ba4a209192779171626a877ee85097R19, this displayed text would not change by any page navigation. For example, if they have the pageLabel as "Page 1", it will be still "Page 1" after they click the Next button because this pageLabel won't change. Just want to confirm that it's expected from the Fusion usage perspective.

@supreethmr
Copy link
Contributor Author

One thing that I want to call out is that, with this change, when the consumer sets a static pageLabel string on the ProgressivePaginator https://github.com/cerner/terra-core/pull/3924/files#diff-5e4d0230ddc94d1702b4ce988ef0f63246ba4a209192779171626a877ee85097R19, this displayed text would not change by any page navigation. For example, if they have the pageLabel as "Page 1", it will be still "Page 1" after they click the Next button because this pageLabel won't change. Just want to confirm that it's expected from the Fusion usage perspective.

That's the expectation of custom page label. user will see value exactly how he provides without any intervention of terra-paginator. ( There might be other use cases where user may require page count in different format than the one that terra-provides. This changes will help user to make those customization )

@chenyixuan625
Copy link
Contributor

Looks like there are a few wdio test failures, could we fix them?

@supreethmr
Copy link
Contributor Author

Looks like there are a few wdio test failures, could we fix them?

fixed the screenshot failures

@supreethmr supreethmr requested a review from sycombs September 26, 2023 18:55
* When specified allows user to set custom page count. User should provide custom page count as part `pageLabel` for best accessibility practices.
* _(usage note: when `pageLabel` is not provided page count will not be hidden and default page count is displayed for best accessibility practices)_.
*/
hidePageCount: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a doc example for how and in what scenario this prop is used? I am having trouble understanding the purpose of this prop based on the test example.

Copy link
Contributor Author

@supreethmr supreethmr Sep 28, 2023

Choose a reason for hiding this comment

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

Thanks @sycombs
Added examples for both custom page label and page count : 5fd4d93

@github-actions github-actions bot temporarily deployed to preview-pr-3924 September 28, 2023 10:41 Destroyed
@supreethmr supreethmr merged commit 3b26f35 into main Sep 30, 2023
@supreethmr supreethmr deleted the paginator-custom-label branch September 30, 2023 15:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants