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

Collection editing revamp #2332

Draft
wants to merge 86 commits into
base: main
Choose a base branch
from
Draft

Conversation

emma-sg
Copy link
Member

@emma-sg emma-sg commented Jan 22, 2025

Closes #2328, #2338

Tentatively ready for review...? This is a lot of changes, I'll split them out into sections below.

Changes

This is a fairly broad pass over the collection editing and sharing experience, focusing primarily on being able to edit collections from anywhere you might want to edit them (i.e. not just from within their detail page).

As part of these changes, we've removed the previous "metadata" editing dialog, and moved name, summary, thumbnail selection (detailed further below), and visibility into a new dialog that can be access from the org dashboard and the collections list, in addition to the collection detail pages.

Back-end changes

We've added a new thumbnailSource field to the Collection model, which preserves the page selected to be collection's thumbnail image. As such, we've separated thumbnail selection from initial replay view selection, though you can still use the initial replay view selection dialog to set the thumbnail if you wish. This field is set via three additional optional query parameters when uploading a thumbnail, and is returned to the client as part of the full collection object.

Front-end changes

This PR has changed quite a bit since the start of it & the call & issue that prompted it, I'd initially taken the route of trying to combine all collection editing interfaces into a single dialog, but along the way we figured out that doing so would require either stacked dialogs, which we decided against, or merging the "select archived items" dialog into the editing dialog, which would be a lot more unwieldy state logic to deal with, and potentially confusing options too coming from different places.

There's a few things left to do still:

  • The current share dialog is still used, and still has visibility & thumbnail selection. This should be removed, and turned into just a dialog with url & embed information.
    Completed in 8eb179d
  • The "edit collection" button should maybe be worded differently — or maybe removed entirely? The dialog it opens can be accessed from the inline edit button on the title/summary already, and from sharing settings
  • We might not get to it this release, but it'd be great to have a button for setting the currently visible page in the replay to the initial replay view
  • Better error handling when selecting thumbnails — we could exclude pages without thumbnails by excluding uploaded pages, it sounds like? Collection editing revamp #2332 (comment)
    Sort of handled 32078be with better error states & messaging, but could be improved on further

@emma-sg emma-sg changed the title Combine collection editing into a single modal dialog Collection editing revamp Jan 28, 2025
@ikreymer
Copy link
Member

Some initial feedback:

  • Since uploads don't have screenshots currently, we could exclude all pages from uploads, though we could support them in the future. Maybe we just say 'Sorry, this page doesn't have a preview' rather than 'couldn't load preview' to be more clear?
  • Find the disabled 'x' a bit confusing, I feel like if I click x I want to discard the changes - that seems like it should just discard and close.

@ikreymer
Copy link
Member

  • Looks like there's still a standalone share button which shows the old share dialog on a new collection. This is redundant with the Sharing tab on Edit Collection:
Screenshot 2025-01-30 at 1 43 31 PM

@emma-sg
Copy link
Member Author

emma-sg commented Jan 30, 2025

Find the disabled 'x' a bit confusing

Sure, yeah, that's fair! Re-enabled it in 77a47ec.

Looks like there's still a standalone share button which shows the old share dialog

Should be addressed in 8eb179d!

Since uploads don't have screenshots currently, we could exclude all pages from uploads

Is there an easy way to do this? The way thing are currently, I don't think there's a way without switching from querying the db to querying RWP, unless I'm mistaken. It'd be great to do though! I'll take a pass at improving the error/not found messaging in the meantime.

@Shrinks99 Shrinks99 linked an issue Feb 1, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants