Skip to content

Replace rebuild docs button with dedicated confirmation page #11508

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 7, 2025
Merged

Replace rebuild docs button with dedicated confirmation page #11508

merged 1 commit into from
Jul 7, 2025
+303 −72

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Jul 3, 2025

Screen Shot 2025-07-03 at 19 04 39

Resolves #11499

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-frontend 🐹 labels Jul 3, 2025
@Turbo87 Turbo87 requested a review from a team July 3, 2025 17:13
GuillaumeGomez
GuillaumeGomez reviewed Jul 3, 2025
View reviewed changes
app/routes/crate/rebuild-docs.js
let user = this.session.currentUser;
let owners = await model.crate.owner_user;
let isOwner = owners.some(owner => owner.id === user.id);
if (!isOwner) {
Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we instead show the message on the page instead of redirecting the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

as mentioned in the DMs, this is how the rest of crates.io behaves and I don't see a reason why we should do it differently for this route. note that the URL will stay the same and the user will see a "This page is only accessible by crate owners" message.

Copy link
Member

Choose a reason for hiding this comment

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

If URL is the same, then all good for me. 👍

@GuillaumeGomez
Copy link
Member

Thanks for the feature!

@Turbo87 Turbo87 force-pushed the wip/docs-rebuild-page branch from a4c977e to df0edad Compare July 5, 2025 15:57
eth3lbert
eth3lbert approved these changes Jul 7, 2025
View reviewed changes
Copy link
Contributor

@eth3lbert eth3lbert left a comment

Choose a reason for hiding this comment

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

LGTM! I'm just not sure if we want the buttons to have a square style with slightly rounded corners, similar to those on the "Add a new Trusted Publisher" or "Report A Crate" pages, instead of the current fully-rounded buttons (not a blocker :D).

@Turbo87
Copy link
Member Author

Turbo87 commented Jul 7, 2025 •
edited
Loading

I'm just not sure if we want the buttons to have a square style with slightly rounded corners, similar to those on the "Add a new Trusted Publisher" or "Report A Crate" pages, instead of the current fully-rounded buttons (not a blocker :D).

I guess either is fine for now, but we should probably settle on one style for the whole page in the future to ensure a somewhat consistent UI 😅

@Turbo87 Turbo87 merged commit 54e756f into rust-lang:main Jul 7, 2025
10 checks passed
@Turbo87 Turbo87 deleted the wip/docs-rebuild-page branch July 7, 2025 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez left review comments

@eth3lbert eth3lbert eth3lbert approved these changes

Assignees
No one assigned
Labels
A-frontend 🐹 C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Turbo87 @GuillaumeGomez @eth3lbert