feat(rbac): upgrade to mui5 #1985
Conversation
|
Thanks for the contribution! |
Changed Packages
|
divyanshiGupta
force-pushed
the
rbac-mui5
branch
2 times, most recently
from
16dcf5b
to
c8d7f05
Compare
Signed-off-by: Divyanshi Gupta <[email protected]>
divyanshiGupta
force-pushed
the
rbac-mui5
branch
from
c8d7f05
to
7beb52c
Compare
| "@material-ui/core": "^4.9.13", | ||
| "@material-ui/icons": "^4.11.3", | ||
| "@material-ui/lab": "^4.0.0-alpha.45", |
| <Button | ||
| className={classes.addRuleButton} | ||
| sx={{ | ||
| display: 'flex !important', | ||
| color: theme => theme.palette.primary.light, | ||
| textTransform: 'none', | ||
| }} | ||
| size="small" | ||
| onClick={handleAddRule} | ||
| > | ||
| <AddIcon fontSize="small" /> | ||
| Add rule | ||
| </Button> |
There was a problem hiding this comment.
I know, this was mostly the same code as before. But instead of custom styles the Button supports this props as well. Can you check if this looks the same:
<Button color="primary" size="small" startIcon={<Add Icon fontSize="small" />}>
Add rule
</Button>If primary color light and primary color isn't the same here, we can maybe anyway use startIcon?
| size="small" | ||
| onClick={handleAddRule} | ||
| > | ||
| <AddIcon fontSize="small" /> | ||
| Add rule | ||
| </Button> | ||
| <Button | ||
| className={classes.addNestedConditionButton} | ||
| sx={{ | ||
| display: 'flex !important', |
There was a problem hiding this comment.
As above, I think we can use the Button prop startIcon instead.
| width: '50vw', | ||
| height: '100vh', | ||
| gap: '3%', | ||
| display: '-webkit-inline-box', |
There was a problem hiding this comment.
I see that you just moved the code. But anyway:
- Is this really necessary?
- Does this work well on Firefox?
| ['& div.MuiDrawer-paper']: { | ||
| width: '0px', | ||
| }, |
There was a problem hiding this comment.
We try to hide what element here?
| <Box style={{ display: 'flex', flexFlow: 'column' }}> | ||
| <Typography className={classes.optionLabel}>{label}</Typography> | ||
| <Typography className={classes.optionDescription}> | ||
| <li |
| @@ -56,9 +59,9 @@ export const AddedMembersTable = ({ | |||
| data={selectedMembers} | |||
| columns={selectedMembersColumns(selectedMembers, setFieldValue)} | |||
| emptyContent={ | |||
| <div className={classes.empty}> | |||
| <EmptyContent className={classes.emptyContent}> | |||
There was a problem hiding this comment.
Can't be this done with a Box like:
| <EmptyContent className={classes.emptyContent}> | |
| <Box sx={{ display: 'flex', justifyContent: 'center', p: 2 }}> |
| style={{ | ||
| display: 'flex', | ||
| flexDirection: 'column', | ||
| alignItems: 'flex-start', | ||
| }} |
There was a problem hiding this comment.
Isn't this the default when using block elements?
Maybe you can remove this custom style by using a div instead of a span below?
| @@ -68,13 +71,13 @@ function DownloadCSVLink() { | |||
| }; | |||
|
|
|||
| return ( | |||
| <a | |||
| <Link | |||
There was a problem hiding this comment.
Can we switch to a Button when this isn't really like a link (cmd+click, right click open in new tab)?
| Unable to create role. | ||
| </AlertTitle> | ||
| To enable create/edit role button, make sure required users/groups are | ||
| available in catalog as a role cannot be created without users/groups | ||
| and also the role associated with your user should have the permission | ||
| policies mentioned{' '} | ||
| <a | ||
| <Link | ||
| href="https://github.com/backstage/community-plugins/tree/main/workspaces/rbac/plugins/rbac#prerequisites" | ||
| target="blank" |
There was a problem hiding this comment.
| target="blank" | |
| target="_blank" |
There is also a Link in @backstage/core-components that handles this automatically.
|
@divyanshiGupta ! Few unit tests are failing , can we fix that . Also I tried running yarn tsc I can see some failure there , too . |
Hey, I just made a Pull Request!
Upgrade RBAC plugin from mui4 to mui5
Story: https://issues.redhat.com/browse/RHIDP-4307
Todo:
Fix unit tests
✔️ Checklist
Signed-off-byline in the message. (more info)