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

Support "Next" and "Previous" button labels in Paginator #710

Closed
MarthinusBosman opened this issue Dec 16, 2022 · 3 comments · Fixed by #714
Closed

Support "Next" and "Previous" button labels in Paginator #710

MarthinusBosman opened this issue Dec 16, 2022 · 3 comments · Fixed by #714
Assignees
Labels
feature request Request a feature or introduce and update to the project.

Comments

@MarthinusBosman
Copy link

MarthinusBosman commented Dec 16, 2022

Describe what feature you'd like. Pseudo-code, mockups, or screenshots of similar solutions are encouraged!

The Paginator component has a few built-in buttons for "previous" and "next" that just have left and right arrows defined as default, with no built in way to override them.

Either have optional slots for previous and next, with the on:click functionality moved to a parent div, or I guess another solution would be to just add optional props to Paginator to specify the label of each button.

This is also an issue for the buttons for Steppers. If there was an opinionated icon system, then fine you could use that, but the literal ↓↑→← arrows should definitely be override-able without doing some sort of workaround.

With class overrides already supported for all these, I think being able to just override the label would be a satisfying solution instead of needing a slot for it, which seems like overkill.

What type of pull request would this be?

Enhancement

Any links to similar examples or other references we should review?

No response

@MarthinusBosman MarthinusBosman added the feature request Request a feature or introduce and update to the project. label Dec 16, 2022
@MarthinusBosman MarthinusBosman changed the title Support "Next" and "Previous" slots in Paginator Support "Next" and "Previous" button labels in Paginator Dec 16, 2022
@endigo9740
Copy link
Contributor

just come off as ugly and should definitely be override-able without doing some sort of workaround

@MarthinusBosman I'm going to ask you keep your feedback kind and constructive. Everyone here is a volunteer. Remember the folks building this are people. Please treat their work with respect. ;)

That said, I agree with your point and I think this is something we can support. We offer similar solutions in other components to support translation to other languages. I don't think this will be a problem to add. I'll also remind you this is open source, so a PR is welcome!

Here's a reference to draw from if someone gets to this before me:

@endigo9740 endigo9740 added good first issue Good for newcomers help wanted Extra attention is needed and removed good first issue Good for newcomers help wanted Extra attention is needed labels Dec 16, 2022
@endigo9740 endigo9740 self-assigned this Dec 16, 2022
@MarthinusBosman
Copy link
Author

Hay sorry, updated the issue to remove "comes off as ugly".

I'll try and get a PR in for this myself tomorrow.

@endigo9740
Copy link
Contributor

No worries, I appreciate the edit. It's my work you were talking about and I've got thick skin heh. But we definitely don't want to discourage other folks.

I'm actually working through small issues this afternoon so I've just updated to include these changes:

Paginator

Screen Shot 2022-12-16 at 3 34 13 PM

Note I've also replaced the hardcoded "Amount" text in the select drop down, so it now read "X Items" by default. Plus I replaced X of Y for the page state with X - Y for better support for i18n and non-English language support.

Stepper

Screen Shot 2022-12-16 at 3 30 36 PM

Additionally I expanded the way the classes are set per buttons on both components, which provides a lot more customization.

These changes be merged into dev branch shortly. Expect to see them in the next release. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request a feature or introduce and update to the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants