fix: confirm delete on Enter in DeleteModal (#918) and cleanup MUI theme logic#970
fix: confirm delete on Enter in DeleteModal (#918) and cleanup MUI theme logic#970Snehadas2005 wants to merge 2 commits intokubeflow:notebooks-v2from
Conversation
…and cleanup MUI theme logic Signed-off-by: Sneha Das <154408198+Snehadas2005@users.noreply.github.com>
google-oss-prow
bot
added
the
area/frontend
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Snehadas2005
changed the title
There was a problem hiding this comment.
Hi @Snehadas2005,
what issues are you experiencing with Material UI? As part of the larger Kubeflow community, we are expected to apply Material UI theme to be compliant when it comes to integrating to the Dashboard. Additionally, the MUI changes are breaking our tests. For these reasons, we kindly ask you open an issue and extract all the MUI related code to another PR.
We can keep the e.preventDefault() in the DeleteModal, however.
| <Form> | ||
| <Form | ||
| onSubmit={(e) => { | ||
| e.preventDefault(); |
|
Hi @thaorell, thank you for the feedback! I understand the requirement for MUI compliance with the Dashboard. I will revert the MUI/Theme cleanup from this branch and keep only the I will update this PR shortly with only the fix for #918. |
Signed-off-by: Sneha Das <154408198+Snehadas2005@users.noreply.github.com>
thaorell
left a comment
thaorell
left a comment
There was a problem hiding this comment.
Hey @Snehadas2005, some of the MUI changes are still present. Please consider removing them from the modal
| import { FlexItem } from '@patternfly/react-core/dist/esm/layouts/Flex'; | ||
| import { HelperText, HelperTextItem } from '@patternfly/react-core/dist/esm/components/HelperText'; | ||
| import { default as ExclamationCircleIcon } from '@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon'; | ||
| import { useThemeContext } from 'mod-arch-kubeflow'; |
| const [inputValue, setInputValue] = useState(''); | ||
| const [isDeleting, setIsDeleting] = useState(false); | ||
| const [error, setError] = useState<string | ApiErrorEnvelope | null>(null); | ||
| const { isMUITheme } = useThemeContext(); |
| <ModalHeader title={title} titleIconVariant="warning" /> | ||
| <ModalBody> | ||
| <Stack hasGutter={!isMUITheme}> | ||
| <Stack hasGutter> |
2 participants
Description
This PR addresses the bug reported in issue #918 regarding the workspace deletion workflow and completes a comprehensive cleanup of defunct MUI theme logic as requested during previous review cycles.
Key Fixes
onSubmithandler withe.preventDefault()to the form, ensuring that hitting Enter triggers the deletion logic directly.isMUIThemeconditionals, and associated styling overrides. These are now handled via mod-arch libraries.Why this new branch (
pf-ui-fix-v2)?This branch provides a clean, consolidated state for these changes. The previous PR (on the
temp3branch) had a complex history due to theme experiments and DCO fix attempts. Starting fresh ensures a clean git history with no lingering merge artefacts or unrelated file changes (fixing the "133 files changed" issue).Detailed Changes
onSubmitto prevent refresh; removeduseThemeContext.ToolbarGrouporder and removed MUI dependencies.MenuToggle..mui-themeselectors and overrides.MUI-theme.scss,ThemeContext.tsx,useThemeContext.ts, andEditableLabels.tsx.How to Test
.mui-themeclasses are applied in the inspector.Checklist