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

[F] Update UI for collaborators #3805

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

[F] Update UI for collaborators #3805

wants to merge 21 commits into from

Conversation

1aurend
Copy link
Contributor

@1aurend 1aurend commented Jan 28, 2025

No description provided.

@1aurend
Copy link
Contributor Author

1aurend commented Feb 11, 2025

@scryptmouse @dananjohnson — this PR review requires a bit of explanation: My initial plan for updating the collaborators UI was to implement a pattern similar to what we've recently done on other projects, where the admin list shows individual contributions. However, as I got into it, there were two key differences: (1) while the Collaborator model already existed, it was never exposed to the client (unless I really missed something). The client sent an array of maker ids for creators and contributors, which the updater handled. (2) position was managed at list-level via a DnD interface rather than as a field that users set on an individual collaborator record. So, after trying a few different setups for the UI, I landed on displaying flattened collaborators in admin as well as in the frontend. It seemed to be the smaller change for users, and I couldn't think of a way to handle position with multiple entries for a single maker in the list, short of changing how position is calculated, doing away with the DnD list, etc. While I like where this ended up, I'm happy to rethink if the endpoints I setup for creating and deleting collaborators by maker are problematic.

For context, here's a screen grab of the updated UI:
https://github.com/user-attachments/assets/ec5fedc1-10ee-4d0d-abf4-ae4fd0ae8dbc

Copy link
Member

@dananjohnson dananjohnson left a comment

Choose a reason for hiding this comment

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

I thought the UI you came up with (as you demoed last week) was solid! I don't have any issues with proceeding with this approach.

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