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

Paginated Label Select Fixes #11510

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Jul 23, 2024

Summary

Fixes #11507

Occurred changes and/or fixed issues

No Options Shown

  • Ensure all ResourceLabeledSelect attributes reach LabelSelect component
  • This worked at some point, but $attrs now does not contain values that are defined as component props

Cannot Load More options

  • Pages param Missing in API response
  • This has been removed, work around added

Search term isn't matching partial results

  • ensure exact filter matching is off

Technical notes summary

  • pages param was on the old api, seeking clarrification if it's never been there on the new api (not sure how i missed it) or it was recently removed
  • vue attrs param was also weird, as this felt like it must have worked when the PR was created, tested and reviewed

Areas or cases that should be tested

Areas which could experience regressions

Checklist

  • The PR is linked to an issue and the linked issue has a Milestone, or no issue is needed
  • The PR has a Milestone
  • The PR template has been filled out
  • The PR has been self reviewed
  • The PR has a reviewer assigned
  • The PR has automated tests or clear instructions for manual tests and the linked issue has appropriate QA labels, or tests are not needed
  • The PR has reviewed with UX and tested in light and dark mode, or there are no UX changes

### No Options Shown
- Ensure all ResourceLabeledSelect attributes reach LabelSelect component
- This worked at some point, but $attrs now does not contain values that are defined as component props

### Cannot `Load More` options
- `Pages` param Missing in API response
- This has been removed, work around added

### Search term isn't matching partial results
- ensure exact filter matching is off
@richard-cox richard-cox added this to the v2.9.0 milestone Jul 23, 2024
@richard-cox richard-cox marked this pull request as ready for review July 23, 2024 12:18
Copy link
Contributor

@aalves08 aalves08 left a comment

Choose a reason for hiding this comment

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

Codewise and testing, LGTM @richard-cox !

Not in the video, but I also tested the partial text search and it worked. I also created a lot of pods (160) and went through the pagination with both server-side pagination on and off and didn't find any problems.

Screen.Recording.2024-07-23.at.15.40.19.mov

@richard-cox richard-cox merged commit e108b95 into rancher:master Jul 23, 2024
31 checks passed
@richard-cox richard-cox deleted the fix-pag-label-select branch July 23, 2024 15:49
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.

Server-side Pagination: Rancher Backups: Credential Secret does not show values
2 participants