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

Improve post_per_page UX #1534

Merged
merged 6 commits into from
Nov 28, 2024
Merged

Improve post_per_page UX #1534

merged 6 commits into from
Nov 28, 2024

Conversation

iansvo
Copy link
Collaborator

@iansvo iansvo commented Nov 25, 2024

@tomusborne
Copy link
Owner

@iansvo I'm still able to save super negative numbers in the posts per page field, is that intended?

I also was never able to type in non-numeric values, even in the current release branch.

@iansvo
Copy link
Collaborator Author

iansvo commented Nov 26, 2024

Are you able to enter in a non-number for a post query or a pro query?

The code here should force that input to be a number with a min of -1, so seems like maybe you're looking at pro. If so you need the related PR.

@iansvo iansvo self-assigned this Nov 26, 2024
@tomusborne
Copy link
Owner

tomusborne commented Nov 27, 2024

@iansvo I'm not able to add a non-numeric value in the release branch, or this branch.

I'm still able to add a value like -12345 in this branch.

This is happening with GB Pro deactivated, and using a regular post query.

@iansvo
Copy link
Collaborator Author

iansvo commented Nov 28, 2024

Ok so apparently we need to handle the minimum in the onChange as well as in the attribute. I also saw that the Control was not quite working correctly with this when I did the onChange fix, which I assume is because the componentValue becomes unsynced with the value prop.

This is now fixed in my latest commit.

@tomusborne tomusborne merged commit 17b3cec into release/2.0.0 Nov 28, 2024
9 checks passed
@tomusborne tomusborne deleted the bug/query-per-page branch November 28, 2024 14:33
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.

2 participants