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

Create Users list details drawer #1687

Merged
merged 10 commits into from
Nov 5, 2024

Conversation

CodyWMitchell
Copy link
Contributor

@CodyWMitchell CodyWMitchell commented Oct 26, 2024

Description

Create Users list details drawer

RHCLOUD-34258


Screenshots

image
image


Checklist ☑️

  • PR only fixes one issue or story
  • Change reviewed for extraneous code
  • UI best practices adhered to
  • Commits squashed and meaningfully named
  • All PR checks pass locally (build, lint, test, E2E)

  • (Optional) QE: Needs QE attention (OUIA changed, perceived impact to tests, no test coverage)
  • (Optional) QE: Has been mentioned
  • (Optional) UX: Needs UX attention (end user UX modified, missing designs)
  • (Optional) UX: Has been mentioned

@CodyWMitchell CodyWMitchell changed the title feat: create user details view Create Users list details drawer Oct 29, 2024
@CodyWMitchell CodyWMitchell marked this pull request as ready for review October 30, 2024 06:42
@karelhala
Copy link
Contributor

/retest

@karelhala karelhala requested a review from a team October 30, 2024 14:37
Comment on lines 40 to 42
const { groups } = useSelector((state: RBACStore) => ({
groups: state.groupReducer?.groups?.data || [],
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const { groups } = useSelector((state: RBACStore) => ({
groups: state.groupReducer?.groups?.data || [],
}));
const groups = useSelector((state: RBACStore) => state.groupReducer?.groups?.data || []);

Comment on lines 85 to 87
const { roles } = useSelector((state: RBACStore) => ({
roles: state.roleReducer?.roles?.data || [],
}));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const { roles } = useSelector((state: RBACStore) => ({
roles: state.roleReducer?.roles?.data || [],
}));
const roles = useSelector((state: RBACStore) => state.roleReducer?.roles?.data || []);

Comment on lines 89 to 101
const fetchData = useCallback(
(apiProps: { username: string }) => {
const { username } = apiProps;
dispatch(fetchRoles({ ...mappedProps({ username }), usesMetaInURL: true, system: false }));
},
[dispatch]
);

useEffect(() => {
fetchData({
username: userId,
});
}, [fetchData, userId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const fetchData = useCallback(
(apiProps: { username: string }) => {
const { username } = apiProps;
dispatch(fetchRoles({ ...mappedProps({ username }), usesMetaInURL: true, system: false }));
},
[dispatch]
);
useEffect(() => {
fetchData({
username: userId,
});
}, [fetchData, userId]);
const fetchData = useCallback(() => {
dispatch(fetchRoles({ ...mappedProps({ username: userId }), usesMetaInURL: true, system: false }));
}, [dispatch, userId]);
useEffect(() => {
fetchData();
}, [fetchData]);

Comment on lines 44 to 56
const fetchData = useCallback(
(apiProps: { username: string }) => {
const { username } = apiProps;
dispatch(fetchGroups({ ...mappedProps({ username }), usesMetaInURL: true, system: false }));
},
[dispatch]
);

useEffect(() => {
fetchData({
username: userId,
});
}, [fetchData, userId]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const fetchData = useCallback(
(apiProps: { username: string }) => {
const { username } = apiProps;
dispatch(fetchGroups({ ...mappedProps({ username }), usesMetaInURL: true, system: false }));
},
[dispatch]
);
useEffect(() => {
fetchData({
username: userId,
});
}, [fetchData, userId]);
const fetchData = useCallback(() => {
dispatch(fetchGroups({ ...mappedProps({ username: userId }), usesMetaInURL: true, system: false }));
}, [dispatch, userId]);
useEffect(() => {
fetchData();
}, [fetchData]);

}));

return (
<div className="pf-v5-u-pt-md">
Copy link
Contributor

Choose a reason for hiding this comment

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

I made myself a note to add support for className in DataView element

<Text>{focusedUser?.email}</Text>
</TextContent>
<DrawerActions>
<DrawerCloseButton onClick={() => onClose()} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<DrawerCloseButton onClick={() => onClose()} />
<DrawerCloseButton onClick={onClose} />

@karelhala
Copy link
Contributor

/retest

Copy link
Contributor

Choose a reason for hiding this comment

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

It might be better to break this file into multiple ones to not have multiple components there

@fhlavac
Copy link
Contributor

fhlavac commented Nov 1, 2024

@CodyWMitchell this should help with the Konflux failure #1690

@karelhala
Copy link
Contributor

/retest

@karelhala
Copy link
Contributor

/retest

1 similar comment
@CodyWMitchell
Copy link
Contributor Author

/retest

Comment on lines 116 to 126
props: {
onClick: () => onFocusUser && onFocusUser(user),
},
Copy link
Contributor

Choose a reason for hiding this comment

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

this is probably not enough.
As mentioned in the DW extension's example isClickable, onRowClick and isRowSelected should be passed
https://react-data-view-pr-data-view-14.surge.sh/extensions/data-view/events-context

Now, the drawer does not hide when you click the row again and the opened row is not focused when clicked. Also the cursor is not a pointer

Copy link
Contributor Author

@CodyWMitchell CodyWMitchell Nov 5, 2024

Choose a reason for hiding this comment

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

updated to use the events-context! 👍
Now the row hides when clicked again, cursor is a pointer, and clicking the actions/checkbox does not trigger the drawer to open. 🎉

red-hat-konflux bot and others added 2 commits November 5, 2024 02:17
Signed-off-by: red-hat-konflux <126015336+red-hat-konflux[bot]@users.noreply.github.com>
Co-authored-by: red-hat-konflux[bot] <126015336+red-hat-konflux[bot]@users.noreply.github.com>
@fhlavac
Copy link
Contributor

fhlavac commented Nov 5, 2024

Looks good! 🎉

@fhlavac fhlavac merged commit d96956a into RedHatInsights:master Nov 5, 2024
10 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.

5 participants