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

feat(manage): improve breadcrumbs usability #2595

Merged

Conversation

wescopeland
Copy link
Member

@wescopeland wescopeland commented Aug 5, 2024

This PR slightly alerts the management app's achievement and leaderboard page breadcrumbs so the game is always in the list of links.

Before
Screenshot 2024-08-05 at 7 05 46 PM

After
Screenshot 2024-08-05 at 7 07 20 PM

@wescopeland wescopeland requested a review from a team August 5, 2024 23:06
@wescopeland wescopeland changed the title feat(manage): improve achievement breadcrumbs usability feat(manage): improve breadcrumbs usability Aug 9, 2024
Copy link
Member

@Jamiras Jamiras left a comment

Choose a reason for hiding this comment

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

Somewhat related: the leaderboard link on the game page goes directly to the edit sub-page.
image

@wescopeland
Copy link
Member Author

wescopeland commented Aug 15, 2024

Somewhat related: the leaderboard link on the game page goes directly to the edit sub-page.

This is a default Filament behavior for users who have the ability to edit the resource. If the user doesn't have the ability to edit the resource, they'll instead be linked to the resource's "Details" (view) page.

With that in mind, we should probably ensure both pages follow a similar layout & show the same fields. I'll make a note to circle back on this in a follow-up PR.

@Jamiras
Copy link
Member

Jamiras commented Aug 15, 2024

This is a default Filament behavior for users who have the ability to edit the resource. If the user doesn't have the ability to edit the resource, they'll instead be linked to the resource's "Details" (view) page.

Odd. The achievement links weren't directed to the edit page for the achievement, and I had permission to edit those too.

@wescopeland
Copy link
Member Author

Ah, that is because in AchievementsRelationManager.php, for some reason I put in an override:

->recordUrl(
    fn (Achievement $record): string => route('filament.admin.resources.achievements.view', ['record' => $record])
)

Now that you mention it, this should probably be removed as it creates an inconsistent user experience.

@wescopeland
Copy link
Member Author

Although, with that override removed, the table rows don't link anywhere at all. That must explain why I added it.

@wescopeland
Copy link
Member Author

I've made a change to those achievement rows in 2c80aa4 so we can have a consistent behavior. The rows now do a policy check to determine what the href should be.

@wescopeland wescopeland merged commit dcfb359 into RetroAchievements:master Aug 16, 2024
5 checks passed
@wescopeland wescopeland deleted the achievement-manage-breadcrumbs branch August 16, 2024 20:26
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