Added Enabled Column in the UserTable #28014
Conversation
Signed-off-by: Kaustubh B <kbawanka@redhat.com>
Signed-off-by: Kaustubh B <kbawanka@redhat.com>
| id={user.id} | ||
| aria-label="User when enabled" | ||
| isChecked={user.enabled} | ||
| hasCheckIcon |
There was a problem hiding this comment.
We haven't been using the hasCheckIcon option. Please remove this to keep consistent with the rest of the app.
There was a problem hiding this comment.
@ssilvert ack , I've pushed the changes.
Signed-off-by: Kaustubh B <kbawanka@redhat.com>
There was a problem hiding this comment.
I though it was decided that we were not going to add a switch here, but rather go with @xianli123's design from the previous PR.
| } | ||
| }; | ||
|
|
||
| // eslint-disable-next-line react/no-unstable-nested-components |
There was a problem hiding this comment.
This rule should not be violated, as it can create performance problems and bugs.
| }), | ||
| ); | ||
| refresh(); | ||
| } catch (error) { |
There was a problem hiding this comment.
Errors should be presented to the user, why are they being swallowed here?
| @@ -391,6 +424,11 @@ export function UserDataTable() { | |||
| displayKey: "firstName", | |||
| cellFormatters: [emptyFormatter()], | |||
| }, | |||
| { | |||
| name: "Enable", | |||
There was a problem hiding this comment.
The name here should use the same casing as other entries.
| name: "Enable", | |
| name: "enable", |
| @@ -391,6 +424,11 @@ export function UserDataTable() { | |||
| displayKey: "firstName", | |||
| cellFormatters: [emptyFormatter()], | |||
| }, | |||
| { | |||
| name: "Enable", | |||
| displayKey: "Enable", | |||
There was a problem hiding this comment.
This needs to refer to a label's key, not a verbatim of what the displayed value should be. Make sure to add a label as well.
| return ( | ||
| <Switch | ||
| id={user.id} | ||
| aria-label="User when enabled" |
There was a problem hiding this comment.
This needs to be localized like other labels. I think we should actually wrap this in a tooltip.
You are right. We decided to remove the @kaustubh-rh Can you make that change? |
@kaustubh-rh We still need to remove the enabled column. If I recall, the original complaint was that the dash in that column was ugly and kind of meaningless. You can modify this PR to do that or we can just close for now. Which do you prefer? |
This PR is continuation of the discussion PR 24569
As suggested by @xianli123 , added the enabled user column. CC : @stianst @ssilvert