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

Replace Mantine modal with Radix dialog #699

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

kmcginnes
Copy link
Collaborator

@kmcginnes kmcginnes commented Dec 4, 2024

Description

This continues the path toward the removal of Mantine and adoption of Radix. This PR replaces all usages of the Mantine component Modal with the Radix equivalent Dialog and AlertDialog.

The method of controlling the open and closed states of the Radix dialog's is a bit different than Mantine Modal and required some finessing. I've done extensive testing on the 8 different modals in the system.

There are also some new style changes I've added. The dialog header now has a consistent style across all dialogs, and some now show a new subtitle. The dialog footer now has a very faint gray background to make it visually distinct from the other elements in the dialog.

For the customize style dialogs I updated the style for the section headings and improved the padding. For the alerts used during the configuration data restore process I had to redesign them a bit to fit with the standard dialog header and footer.

There are before and after shots in the validation section below.

Validation

  • Ensured dialog triggers behave properly
  • Ensured dialogs scroll when view port is too small
  • Ensured escape and click outside closes Dialog but not AlertDialog

Connection UI & Custom SPARQL Namespace

Not much different here. Just the new standard header and footer styles.

Before After
Connection dialog before Connection dialog after
Custom namespace dialog before Custom namespace dialog after

Configuration Restore

This required the most redesign of all.

  • Uses Dialog for the first dialog since it can be canceled and escaped easily
  • Uses AlertDialog for the success and error screens since they are informational and only have a single option for the user to choose
  • Uses icons from Lucide to reduce visual weight
  • Adds a "well" design and an icon for the file name in the confirmation and error dialogs to make it more visually distinct
Before After
Configuration restore confirmation before Configuration restore confirmation after
Configuration restore success before Configuration restore success after
Configuration restore failure before Configuration restore failure after

Customize vertex or edge styles

  • Adjusted spacing between sections
  • Updated style of section header to give it more prominence
  • Adjusted title & added subtitle
Before After
Customize vertex style before Customize vertex style after
Customize edge style before Customize edge style after

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0
    license.
  • I have run pnpm checks to ensure code compiles and meets standards.
  • I have run pnpm test to check if all tests are passing.
  • I have covered new added functionality with unit tests if necessary.
  • I have added an entry in the Changelog.md.

Copy link

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 0% with 1114 lines in your changes missing coverage. Please review.

Project coverage is 15.86%. Comparing base (9d3a2b7) to head (04cbc53).
Report is 46 commits behind head on main.

Files with missing lines Patch % Lines
...rer/src/modules/EdgesStyling/SingleEdgeStyling.tsx 0.00% 158 Missing ⚠️
...rer/src/modules/NodesStyling/SingleNodeStyling.tsx 0.00% 152 Missing and 1 partial ⚠️
.../src/modules/CreateConnection/CreateConnection.tsx 0.00% 145 Missing ⚠️
...lorer/src/workspaces/Settings/LoadConfigButton.tsx 0.00% 131 Missing ⚠️
...ckages/graph-explorer/src/components/NewSelect.tsx 0.00% 109 Missing and 1 partial ⚠️
packages/graph-explorer/src/components/Dialog.tsx 0.00% 96 Missing and 1 partial ⚠️
...ages/graph-explorer/src/components/AlertDialog.tsx 0.00% 94 Missing and 1 partial ⚠️
...h-explorer/src/modules/Namespaces/UserPrefixes.tsx 0.00% 81 Missing ⚠️
...orer/src/components/PanelEmptyState/EmptyState.tsx 0.00% 62 Missing and 1 partial ⚠️
packages/graph-explorer/src/components/Panel.tsx 0.00% 18 Missing ⚠️
... and 10 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #699      +/-   ##
==========================================
+ Coverage   14.88%   15.86%   +0.98%     
==========================================
  Files         566      540      -26     
  Lines       25121    23450    -1671     
  Branches     1170     1133      -37     
==========================================
- Hits         3739     3721      -18     
+ Misses      21050    19411    -1639     
+ Partials      332      318      -14     
Flag Coverage Δ
unittests 15.86% <0.00%> (+0.98%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kmcginnes kmcginnes marked this pull request as ready for review December 4, 2024 19:00
@kmcginnes kmcginnes marked this pull request as draft December 6, 2024 16:48
@kmcginnes
Copy link
Collaborator Author

Bug found where the select menu items can not be selected. It might be possible that I need to convert them to use the new select component because of the use of React Portals in the new Dialog component.

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.

[Task] Migrate Modal to Radix & Tailwind
1 participant