dspace-angular icon indicating copy to clipboard operation
dspace-angular copied to clipboard

Replace dso page edit buttons with a menu

Open YanaDePauw opened this issue 2 years ago • 5 comments

Description

This PR removes the separate buttons from the various DSpace object pages, such as the edit button and the versioning button, and puts them in a menu which will be rendered as buttons.

To add new buttons to the edit pages, these can be added to the menus in the DSOEditMenuResolver, similar to the general MenuResolver.

Every top level menu will be rendered as a button. When this menu has no child menus, clicking the button will perform the action defined in the menu section in the resolver. When the menu has child menus, clicking the button will show a dropdown containing the actions defined in the child menu sections.

Furthermore, because the version button on the item page can be disabled, the menu functionality was expanded with a disabled option. Support for this disabled property was added to all other existing menus, but is currently not in use.

Instructions for Reviewers

The changes were made in such a way that the UI remains the same for the different dspace object pages. To verify this, you can do the following:

  • As an anonymous user or a user that does not have edit rights:
    • Navigate to some community, collection and item pages
    • Verify that you do not see any edit buttons
  • As an admin user or a user authorised to edit dspace objects:
    • Navigate to some community, collection and item pages
    • Verify that you can see the edit button
    • Verify that you can see the item version button on the item page and that it behaves as before

Checklist

  • [x] My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
  • [x] My PR passes TSLint validation using yarn run lint
  • [x] My PR doesn't introduce circular dependencies
  • [x] My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
  • [x] My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
  • [x] If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

YanaDePauw avatar Sep 22 '22 14:09 YanaDePauw

@YanaDePauw : This has merge conflicts. Could you rebase this on main when you get the chance?

tdonohue avatar Sep 26 '22 19:09 tdonohue

This pull request introduces 1 alert when merging 6671bb3eb04db2e2cbf2802bf5bfd85b87af6aa1 into 064dae258151bdd93e47c9bc3a0321ed86fe2063 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Sep 27 '22 14:09 lgtm-com[bot]

@tdonohue Sorry for pushing more changes this late, but we wanted to sneak in

  • Support for optional icons within dropdown menus
  • A fix for the width of the dropdown (it used to be very narrow, which caused issues for links with long titles)

Here's a quick branch demonstrating the dropdowns: w2p-94390_replace-dso-page-edit-buttons-with-a-menu_Testing

ybnd avatar Sep 28 '22 17:09 ybnd

This pull request introduces 2 alerts when merging 9dc7b3490fff3a77c752b5d31f46320ccc101238 into 695ce3ab9e7a2b84b4ad66a71fbf84848b623479 - view on LGTM.com

new alerts:

  • 2 for Unused variable, import, function or class

lgtm-com[bot] avatar Sep 28 '22 17:09 lgtm-com[bot]

This pull request introduces 1 alert when merging 3c9a5e8c5529519e72fb9c37d8750ec0fbcccec1 into 273c7543707ccdad44c7fcafcfa967c1fd0b98e9 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

lgtm-com[bot] avatar Oct 11 '22 16:10 lgtm-com[bot]

Hi @YanaDePauw

is working well for me, in article, community and collection. image

jtimal avatar Nov 04 '22 20:11 jtimal

@tdonohue @YanaDePauw I have some doubts on where the logic to build the menu is done. Essentialy now it's all delegated to the dso-edit-menu.resolver including specific functions used by a menu entry (see claim research action) So I'm worry that if the menu options will increase it could became a bit confusing.

@YanaDePauw's solution makes the menu on DSO pages work the exact same way as all other menus in DSpace. If we merge this PR, and find that menus need to be more flexible later on, we can improve all menus at once then. I'd prefer that to replacing one custom solution for DSO page menus with another

artlowel avatar Nov 10 '22 16:11 artlowel

This pull request introduces 1 alert when merging 4d297cfe9b130f36d87d3f3f8958e8b27b22cef8 into 89447a6c32ee1fd28d9f2c5e523becdb8622d734 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine :gear: that powers LGTM.com. For more information, please check out our post on the GitHub blog.

lgtm-com[bot] avatar Nov 17 '22 14:11 lgtm-com[bot]

@tdonohue

@YanaDePauw recently noticed that DSO page menu sections with OnClick callbacks break in production mode, so this PR probably shouldn't be merged as-is.

We're looking into it.

ybnd avatar Nov 29 '22 19:11 ybnd

Hi @YanaDePauw, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

github-actions[bot] avatar Dec 07 '22 16:12 github-actions[bot]

@atarix83 : I believe the alignment is horizontal unless there's a very long title. That's the same behavior as we currently have on main. See that same item in the demo site: https://demo7.dspace.org/entities/publication/0ef86789-838c-424e-9585-0f917fbfea6a After logging in to the demo site, the buttons will appear vertical in the same way as your screenshot. However, if you visit an item with a shorter title, the button alignment will be horizontal.

So, as far as I can tell, I believe the behavior in this PR is identical to the behavior on main. Buttons are horizontal, unless the title is so long that it forces them to be vertical.

tdonohue avatar Jan 17 '23 15:01 tdonohue

@tdonohue

I think we can fix in this PR

@YanaDePauw it's just enough to remove the flex-wrap here https://github.com/atmire/dspace-angular/blob/w2p-94390_replace-dso-page-edit-buttons-with-a-menu/src/app/shared/dso-page/dso-edit-menu/dso-edit-menu.component.html#L1

and here the result Schermata da 2023-01-19 10-54-33

atarix83 avatar Jan 19 '23 11:01 atarix83

Hi @YanaDePauw, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

github-actions[bot] avatar Jan 24 '23 15:01 github-actions[bot]

Hi @YanaDePauw, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

github-actions[bot] avatar Feb 07 '23 20:02 github-actions[bot]

Hi @YanaDePauw, Conflicts have been detected against the base branch. Please resolve these conflicts as soon as you can. Thanks!


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

github-actions[bot] avatar Feb 09 '23 23:02 github-actions[bot]