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

Remix projects access requests actions dropdown #5155

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

ruggi
Copy link
Contributor

@ruggi ruggi commented Apr 2, 2024

Fix #5140

Builds on top of #5139 and #5142 .

Problem:

It should be possible to manipulate access requests for collaborative projects besides the basic approve of a new request.

Fix:

Pending request Existing request
Screenshot 2024-04-02 at 11 37 21 AM Screenshot 2024-04-02 at 11 38 26 AM

This PR adds a dropdown to the sharing dialog, per collaborator row. The dropdown allows the user to approve, reject, or discard access requests. The different options change depending on the current status of the request. For pending requests, the button has a yellow-orange accent background.

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Try me

Copy link

relativeci bot commented Apr 2, 2024

Job #11383: Bundle Size — 63.39MiB (~-0.01%).

c23c9b5(current) vs 2296c28 master#11381(baseline)

Warning

Bundle contains 58 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
Job #11383
     Baseline
Job #11381
Regression  Initial JS 50.43MiB(~+0.01%) 50.43MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 18.88% 22.25%
No change  Chunks 41 41
No change  Assets 45 45
No change  Modules 4469 4469
No change  Duplicate Modules 608 608
No change  Duplicate Code 32.05% 32.05%
No change  Packages 467 467
No change  Duplicate Packages 58 58
Bundle size by type  Change 2 changes Regression 1 regression Improvement 1 improvement
                 Current
Job #11383
     Baseline
Job #11381
Regression  JS 63.38MiB (~+0.01%) 63.38MiB
Improvement  HTML 14.15KiB (-0.44%) 14.21KiB

View job #11383 reportView feat/share-access-dropdown branch activityView project dashboard

Copy link
Contributor

github-actions bot commented Apr 2, 2024

Performance test results:
(Chart1)
(Chart2)

// the access requests, including in-flight optimistic statuses
const [accessRequests, setAccessRequests] = React.useState(props.accessRequests)
// the last successfully-obtained access requests that can be used to roll-back in case of issues when updating requests
const [stableAccessRequests, setStableAccessRequests] = React.useState(props.accessRequests)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be called previousAccessRequests

return (
<DropdownMenu.Root>
<DropdownMenu.Trigger>
{/* this needs to be inlined (and as a ternary) because DropdownMenu.Trigger requires a direct single child */}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should pull it out into its own component in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah my comment is meant for that case too: it needs an explicit inline child, because otherwise it will explode due to the component not accepting refs (?!). You can see I tried doing it here but had to revert it back :( 645b73a (#5155)

@ruggi ruggi merged commit 118354b into master Apr 2, 2024
15 checks passed
@ruggi ruggi deleted the feat/share-access-dropdown branch April 2, 2024 15:01
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.

Share dialog access requests dropdown
4 participants