Skip to content

Use action group extensions for group/submenus and create new action menu components using PF menu#9365

Merged
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
rohitkrai03:action-groups
Jul 2, 2021
Merged

Use action group extensions for group/submenus and create new action menu components using PF menu#9365
openshift-merge-robot merged 3 commits intoopenshift:masterfrom
rohitkrai03:action-groups

Conversation

@rohitkrai03
Copy link
Copy Markdown
Contributor

@rohitkrai03 rohitkrai03 commented Jun 28, 2021

Fixes:

Analysis / Root cause:

  • The ActionGroup extension was not used to create groups and submenus in kebab.
  • PF components were not being used in KebabMenu.

Solution Description:

  • Added new logic in ActionLoader that uses ActionGroup extensions to create menu options with groups and submenus.
  • Created new ActionMenu components that uses PF menu components. Had to use custom menu and submenu rendering logic due to PF bug with submenus.
  • Added logic to sort all the actions based on insertBefore and insertAfter properties.
  • ActionMenu component now supports both kebab as well as dropdown variant that can be used for different action menu scenarios.
  • Migrated actions menu on the helm details page to use new extension based actions and ActionsLoader.

Screen shots / Gifs for design review:

Screen.Recording.2021-06-29.at.3.06.38.AM.mov

Unit test coverage report:

Screenshot 2021-06-29 at 9 10 15 PM

Screen.Recording.2021-06-29.at.9.58.12.PM.mov

Test setup:

  • Added a dummy commit that updates the helm actions to use dummy groups and submenus. This commit is just for testing and will be deleted before merging.

Browser conformance:

  • Chrome
  • Firefox
  • Safari
  • Edge

@openshift-ci openshift-ci bot added component/helm Related to helm-plugin component/shared Related to console-shared approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jun 28, 2021
@openshift-ci openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Jun 29, 2021
@openshift-ci openshift-ci bot added the component/core Related to console core functionality label Jun 29, 2021
@rohitkrai03
Copy link
Copy Markdown
Contributor Author

/assign @christianvogt

Copy link
Copy Markdown
Contributor

@sahil143 sahil143 left a comment

Choose a reason for hiding this comment

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

Looks Good to me.

Tested Locally and works as expected.

I only see one issue with the component is that clicking on subgroup closes the dropdown and action is not performed.

@rohitkrai03 rohitkrai03 force-pushed the action-groups branch 2 times, most recently from 822d0b8 to 7814209 Compare July 2, 2021 11:42
@rohitkrai03
Copy link
Copy Markdown
Contributor Author

rohitkrai03 commented Jul 2, 2021

@sahil143 Deleted the dummy commit. The issue with sub menu clicks is because of popper rendering popper as submenus. Since there are no actual use cases for submenus right now, we'll wait for this PF fix - patternfly/patternfly-react#5789. Once the PF fix is in we can update our code to use PF Menu component directly. I think we can get this PR merged if there are no other comments because #9306 also depends on this PR.

@sahil143
Copy link
Copy Markdown
Contributor

sahil143 commented Jul 2, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2021
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci bot commented Jul 2, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rohitkrai03, sahil143

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 7be2eda into openshift:master Jul 2, 2021
@spadgett spadgett added this to the v4.9 milestone Jul 14, 2021
@rohitkrai03 rohitkrai03 deleted the action-groups branch July 24, 2021 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality component/helm Related to helm-plugin component/shared Related to console-shared kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants