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

Show Role Audience of Assigned Roles of the Group #4688

Merged
merged 4 commits into from
Nov 19, 2023

Conversation

Shenali-SJ
Copy link
Contributor

@Shenali-SJ Shenali-SJ commented Nov 18, 2023

Purpose

This resolves the issue of not displaying the role audience in the assigned roles of a specific group.

Related Issues

Related PRs

Revised UI

Screenshot 2023-11-18 at 22 05 15

Checklist

  • e2e cypress tests locally verified.
  • Manual test round performed and verified.
  • UX/UI review done on the final implementation.
  • Documentation provided. (Add links if there are any)
  • Unit tests provided. (Add links if there are any)
  • Integration tests provided. (Add links if there are any)

Security checks

@@ -31,17 +30,21 @@ interface RenderChipInterface extends IdentifiableComponentInterface, ChipProps
*/
setActiveOption: (option: RolesMemberInterface) => void;
/**
* Primary text of the chip.
* Display name of the role
Copy link
Contributor

Choose a reason for hiding this comment

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

If we are updating the component properties specifically for displaying role information, let's also rename RenderChip and related interface names as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8cb8bf2

} = props;

const [ popoverAnchorEl, setPopoverAnchorEl ] = useState<Element>(null);
const [ , setPopoverAnchorEl ] = useState<Element>(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we remove this as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8cb8bf2

@Achintha444
Copy link
Contributor

Can you add a screenshot on how it will display when assigned multiple org roles.

@@ -118,7 +118,10 @@ export const EditGroupRoles: FunctionComponent<EditGroupRolesPropsInterface> = (
<RenderChip
{ ...getTagProps({ index }) }
key={ index }
primaryText={ option.display }
primaryText={ "" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove primaryText prop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8cb8bf2

} = props;

const [ popoverAnchorEl, setPopoverAnchorEl ] = useState<Element>(null);
const [ , setPopoverAnchorEl ] = useState<Element>(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@@ -19,7 +19,6 @@
import Chip, { ChipProps } from "@oxygen-ui/react/Chip";
import { IdentifiableComponentInterface, RolesMemberInterface } from "@wso2is/core/models";
import React, { FunctionComponent, ReactElement, SyntheticEvent, useState } from "react";
import { ChipMoreDetails } from "./chip-more-details";
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove chip-more-details file as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with 8cb8bf2

@Shenali-SJ
Copy link
Contributor Author

The following screenshot displays how the roles will be shown when there are multiple org audience roles.
image

@pavinduLakshan pavinduLakshan merged commit e762ac1 into wso2:master Nov 19, 2023
5 checks passed
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.

3 participants