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

User view: always show prev-next #6610

Merged
merged 1 commit into from
Aug 31, 2024

Conversation

distantnative
Copy link
Member

@distantnative distantnative commented Aug 11, 2024

Description

Summary of changes

  • Prev/next arrows are shown on any user view, also the account view
  • $user->next(), $user->prev() are based on a siblings collection that now is sorted by username (not id)

Reasoning

  • Current state has been too confusing. Maybe the new solution isn't the best one can imagine, but I'd argue better and a lot less confusing.
  • Prev/next arrows should move through the same list as shown on users view. That's why the sorting needs to be added to User::siblingsCollection() too.

Additional context

  • I think we can improve this a lot more easily once we have view UI classes. That will allow us e.g. to have specific routes for the user view but when filtered by a specific role, where the prev-next arrows could then loop only through the users with that role.

Changelog

Enhancements

  • User view: show prev/next buttons also on account view

For review team

  • Add changes & docs to release notes draft in Notion

@distantnative distantnative added the type: enhancement ✨ Suggests an enhancement; improves Kirby label Aug 11, 2024
@distantnative distantnative added this to the 4.4.0 milestone Aug 11, 2024
@distantnative distantnative self-assigned this Aug 11, 2024
@distantnative distantnative marked this pull request as ready for review August 11, 2024 20:29
@distantnative distantnative requested a review from a team August 11, 2024 20:29
@bastianallgeier
Copy link
Member

I think the most confusing part is that we jump to the account view for the current user. We should probably separate this and have an account view and a user view for the current user.

@distantnative
Copy link
Member Author

@bastianallgeier I think I found a relatively simple way that seems to work

@bastianallgeier
Copy link
Member

The option idea is great. But unfortunately, we can't simply change the path method like that. It's needed in other places as well. I guess it's best to untangle that as well in this PR.

@distantnative distantnative modified the milestones: 4.4.0, 4.5.0 Aug 13, 2024
@distantnative distantnative force-pushed the enhancement/4225-user-prevnext-always branch from 347794e to 093ef2e Compare August 15, 2024 09:58
@distantnative
Copy link
Member Author

@bastianallgeier removed that part again. I still think that this PR improves the situation, even if it's not the perfect result.

@afbora
Copy link
Member

afbora commented Aug 15, 2024

I like this solution. It's easy and simple 👍

If we're still not sure and it confusing, I have an another idea that remove current user from users collection only for the pagination. Maybe it's stupid idea 🙈

@distantnative distantnative merged commit 4caf88f into develop-minor Aug 31, 2024
15 checks passed
@distantnative distantnative deleted the enhancement/4225-user-prevnext-always branch August 31, 2024 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement ✨ Suggests an enhancement; improves Kirby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logical inconsistency while navigating through panel users (prev/next buttons disappear on own account)
3 participants