Skip to content

Conversation

vr-varad
Copy link
Contributor

Notes for Reviewers

This PR fixes keeps all changes for themed Modal to sistent.

Signed commits

  • Yes, I signed my commits.

Signed-off-by: vr-varad <[email protected]>
@vr-varad vr-varad added the pr/do-not-merge PRs not ready to be merged label Jun 20, 2025
@leecalcote
Copy link
Member

@vr-varad get people to review. Neither @FaheemOnHub nor @vishalvivekm have responded here for the past 24 hours, so you’re going to have to chase them down.

vr-varad added 3 commits June 22, 2025 15:07
Signed-off-by: vr-varad <[email protected]>
Signed-off-by: vr-varad <[email protected]>
Signed-off-by: vr-varad <[email protected]>
@vr-varad
Copy link
Contributor Author

@FaheemOnHub @vishalvivekm

Copy link
Contributor

@FaheemOnHub FaheemOnHub left a comment

Choose a reason for hiding this comment

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

this approach of passing the style to sistent modals , looks good as theme colors are not defined in sistent and also the changes are minor.

@@ -68,6 +68,9 @@ interface UserInviteModalProps {
};
isAssignUserRolesAllowed: boolean;
useLazyGetTeamsQuery: any;

/* eslint-disable @typescript-eslint/no-explicit-any */
Copy link
Member

Choose a reason for hiding this comment

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

Why is this best practice being ignored?

: darkModalGradient.fotter
? theme.palette.mode === 'light'
? lightModalGradient.fotter
: darkModalGradient.fotter
Copy link
Member

Choose a reason for hiding this comment

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

Footer is misspelled?

@leecalcote
Copy link
Member

This approach isn't ideal.

@leecalcote leecalcote closed this Jun 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/do-not-merge PRs not ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants