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

[#55163] Build project-list sharing modal #15971

Merged
merged 68 commits into from
Jul 4, 2024

Conversation

aaron-contreras
Copy link
Contributor

@aaron-contreras aaron-contreras commented Jun 25, 2024

ToDo List

  • Add ProjectQuery as a shareable resource
  • Build the contracts to share ProjectQuery
  • Add a ShareDialog that will be added as an async dialog via Turbo
  • Introduce a SharingStrategy pattern that wraps all of the content of the methods sharing_contract_scope, available_roles and potentially more to not end up with big case statements in the SharesController as suggested by @ulferts in Generalize sharing modal #15899 (comment)
  • Change the Sharing modal to allow injecting components
    • Inject a body component that adds the switch for making the query public or removing it (when allowed)
    • Inject a custom empty state that shows a different message when there are no shares, but the query is marked as public
      • Nice to have: When in an empty state and toggling the query between public/private, the empty state text should be re-rendered without having to close and re-open the modal.
  • Currently the sharing controller is only accessible when we have the share_work_packages permission, we probably need to remove all permission checks from the shares controller and then handle permission checking with the manageable? method in the strategy and contracts.
  • can_save?, can_rename? checks for the project query index header component need to be changed to use the visbile? and editable? methods on the Query (@klaustopher)
  • Role names and descriptions should come from i18n files instead of directly from the DB (copy what we did for work package roles)
    • Add role description translations
  • If you cannot view shares, hide the share button
  • Check all i18n strings
  • Show warning about project access

Currently identified problems

  • Fix CSS top-layer issue with invite user form
  • Fix CSS issues with the modal
    • The ActionMenu Primer component uses an overlay to render its menu item list which ends up causing the container to span the entire width of the dialog for some reason
  • The modal currently closes after each action that is taken in the modal when sharing ProjectQueries

Documentation TODOs


See: https://community.openproject.org/work_packages/55163

@aaron-contreras aaron-contreras added this to the 14.3.x milestone Jun 25, 2024
@aaron-contreras aaron-contreras self-assigned this Jun 25, 2024
@aaron-contreras aaron-contreras changed the title [#55163] Build project-list-sharing modal [#55163] Build project-list sharing modal Jun 25, 2024
@klaustopher
Copy link
Contributor

When we add #15983 here, the modal already works in a super rough capacity

@aaron-contreras aaron-contreras force-pushed the implementation/55163-build-sharing-modal branch from 38c8adb to 6dafc14 Compare June 27, 2024 02:13
@aaron-contreras
Copy link
Contributor Author

Rebased onto #15983 in order to get the fixes on there over on this branch

@aaron-contreras
Copy link
Contributor Author

Current modal rendering state

Known issues:

  • Action Menu rendering inside the modal spans way too wide. I suspect this has something to do with the CSS top layer.
Screen.Recording.2024-06-26.at.21.54.28.mov

@klaustopher
Copy link
Contributor

#15983 has been merged and the generalize-sharing-modal branch is based on current dev, so I am rebasing this branch bakc onto generalize sharing modal, please pull with. git fetch --all && git reset --hard origin/implementation/55163-build-sharing-modal

@klaustopher klaustopher force-pushed the implementation/55163-build-sharing-modal branch from 5fb4649 to 48a3aec Compare June 27, 2024 07:53
@klaustopher klaustopher mentioned this pull request Jun 27, 2024
12 tasks
Base automatically changed from generalize-sharing-modal to dev June 27, 2024 19:03
@HDinger
Copy link
Contributor

HDinger commented Jul 1, 2024

Current modal rendering state

Known issues:

* [ ]  Action Menu rendering inside the modal spans way too wide. I suspect this has something to do with the CSS top layer.

Screen.Recording.2024-06-26.at.21.54.28.mov

This is actually an issue of Primer itself where the automatic width calulation of ActionMenus inside Dialogs do not work, see primer/view_components#2926. As a wrokaround for now, we have to enforce the width of the ActionMenu manually. Unfortnately, we thus have to accept that the width is not always ideal.

@klaustopher klaustopher force-pushed the implementation/55163-build-sharing-modal branch from dbc0580 to 637ae9f Compare July 3, 2024 07:37
@klaustopher klaustopher force-pushed the implementation/55163-build-sharing-modal branch from 637ae9f to a7403fe Compare July 3, 2024 07:38
@klaustopher klaustopher marked this pull request as ready for review July 3, 2024 09:42
Copy link
Contributor

@dombesz dombesz left a comment

Choose a reason for hiding this comment

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

Very nice job @aaron-contreras and @klaustopher , just 2 minor things. Feel free to merge.

lookbook/docs/patterns/05-dialogs.md.erb Outdated Show resolved Hide resolved
Copy link
Contributor

@toy toy left a comment

Choose a reason for hiding this comment

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

Mostly nitpicks, though no specs added

app/components/shares/empty_state_component.rb Outdated Show resolved Hide resolved
app/components/shares/modal_body_component.html.erb Outdated Show resolved Hide resolved
app/components/shares/modal_body_component.rb Outdated Show resolved Hide resolved
app/menus/projects/menu.rb Outdated Show resolved Hide resolved
app/models/sharing_strategies/base_strategy.rb Outdated Show resolved Hide resolved
app/views/shares/dialog.turbo_stream.erb Outdated Show resolved Hide resolved
@klaustopher klaustopher changed the base branch from dev to release/14.3 July 4, 2024 06:14
@klaustopher klaustopher merged commit 3464147 into release/14.3 Jul 4, 2024
10 checks passed
@klaustopher klaustopher deleted the implementation/55163-build-sharing-modal branch July 4, 2024 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 participants