Skip to content

ILEX-99 Revisit UI elements for term addition#125

Merged
ddelpiano merged 24 commits intodevelfrom
feature/ILEX-99
Sep 1, 2025
Merged

ILEX-99 Revisit UI elements for term addition#125
ddelpiano merged 24 commits intodevelfrom
feature/ILEX-99

Conversation

@Aiga115
Copy link

@Aiga115 Aiga115 commented Aug 20, 2025

Issue #ILEX-99

Problem: Revisit UI elements for term addition

Solution:

  1. Make change to ui of add new term dialog
  2. Add button group component to trigger dialog
  3. Refactor the ways how we use inputs

Note: Logic of creating new term is not completed yet so it is not really possible to see the second step for now, but UI is ready

Result:

Recording.2025-08-20.151354.mp4
Recording.2025-08-20.151817.mp4

@Aiga115 Aiga115 requested a review from ddelpiano August 20, 2025 13:19
@Aiga115 Aiga115 marked this pull request as draft August 20, 2025 13:23
@Aiga115 Aiga115 marked this pull request as ready for review August 20, 2025 14:12
@jrmartin jrmartin requested a review from Copilot August 27, 2025 15:08
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors UI elements for term addition by consolidating multiple input components into a single CustomFormField component, adding a new CustomButtonGroup component, and updating the mobile stepper theme. The changes aim to standardize form inputs and improve the term addition dialog UI.

  • Replaces multiple input components (CustomizedInput, CustomInputBox) with a unified CustomFormField
  • Introduces CustomButtonGroup component for dropdown button functionality
  • Updates theme styling for ButtonGroup and MobileStepper components

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/theme/index.jsx Updates MuiButtonGroup and MuiMobileStepper theme styles
src/components/common/CustomizedInput.jsx Removes deprecated input component
src/components/common/CustomInputBox.jsx Removes deprecated input component
src/components/common/CustomFormField.jsx Adds new unified form field component
src/components/common/CustomButtonGroup.jsx Adds new button group component with dropdown menu
src/components/TermEditor/newTerm/* Implements new term addition dialog components
Various other files Updates component imports and prop names to use CustomFormField

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +198 to +199
label: PropTypes.string.isRequired,
name: PropTypes.string.isRequired,
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The PropTypes validation requires 'label' and 'name' as required props, but the component code shows 'label' is optional (line 130 checks 'if (label)') and 'name' is not used in the component. The PropTypes should match the actual component implementation.

Suggested change
label: PropTypes.string.isRequired,
name: PropTypes.string.isRequired,
label: PropTypes.string,

Copilot uses AI. Check for mistakes.
variant="body2"
sx={{
marginTop: "0.375rem",
color: isError ? "#F04438" : gray600
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The error color '#F04438' is hardcoded instead of using the theme variable. This should use the 'error600' or similar variable from the theme for consistency.

Suggested change
color: isError ? "#F04438" : gray600
color: isError ? error600 : gray600

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

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

@Aiga115 this is a valid point from copilot, thanks!

@jrmartin
Copy link

@Aiga115 Sorry , I accidentally asked copilot to review the wrong branch -_-

ddelpiano added a commit that referenced this pull request Aug 29, 2025
#125- Remove header with token authorization for POST ontology, not needed anymore
@ddelpiano ddelpiano merged commit 80a95d6 into devel Sep 1, 2025
1 check passed
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.

3 participants