-
Notifications
You must be signed in to change notification settings - Fork 21
feat(admin-ui): revamp Scripts module as per Figma with partial TS migration #2680
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
Changes from all commits
e90dbd9
66f276e
690ae74
77679c7
65fc043
79b490a
b5dd4de
23758ed
3756ce5
2c6b5a1
c4789c7
b344e8b
a59cf27
be68a84
2d8ff15
6472183
c6f19a1
b851490
e823740
effbcbf
6584a16
d218eca
c8c5cd4
3617e5c
18a6c6d
e3aeaac
88f6b40
e635bb3
118549c
faaf9b9
3da01a2
b68889c
e70a944
6a5363b
d896391
9f680ef
4dbdb74
7ffb66e
9b01976
d65a24d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,21 +1,16 @@ | ||
| // @ts-nocheck | ||
| import React from 'react' | ||
| import PropTypes from 'prop-types' | ||
| import classNames from 'classnames' | ||
| import { Collapse, CardBody } from 'reactstrap' | ||
|
|
||
| import { Consumer } from './context' | ||
| import type { AccordionBodyProps } from './Accordion.d' | ||
|
|
||
| export const AccordionBody = (props) => ( | ||
| export const AccordionBody: React.FC<AccordionBodyProps> = ({ children, className }) => ( | ||
| <Consumer> | ||
| {({ isOpen }) => ( | ||
| <Collapse isOpen={isOpen}> | ||
| <CardBody className={classNames(props.className, 'pt-0')}>{props.children}</CardBody> | ||
| <CardBody className={classNames(className, 'pt-0')}>{children}</CardBody> | ||
| </Collapse> | ||
| )} | ||
| </Consumer> | ||
| ) | ||
| AccordionBody.propTypes = { | ||
| children: PropTypes.node, | ||
| className: PropTypes.string, | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,15 @@ | ||||||||||||||||||||||||||||||||||||
| // @ts-nocheck | ||||||||||||||||||||||||||||||||||||
| import { Accordion } from './Accordion' | ||||||||||||||||||||||||||||||||||||
| import { Accordion as AccordionBase } from './Accordion' | ||||||||||||||||||||||||||||||||||||
| import { AccordionBody } from './AccordionBody' | ||||||||||||||||||||||||||||||||||||
| import { AccordionHeader } from './AccordionHeader' | ||||||||||||||||||||||||||||||||||||
| import { AccordionIndicator } from './AccordionIndicator' | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| type AccordionComponent = typeof AccordionBase & { | ||||||||||||||||||||||||||||||||||||
| Body: typeof AccordionBody | ||||||||||||||||||||||||||||||||||||
| Header: typeof AccordionHeader | ||||||||||||||||||||||||||||||||||||
| Indicator: typeof AccordionIndicator | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| const Accordion = AccordionBase as AccordionComponent | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+12
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Prefer the shared This file redefines a type that already exists in ♻️ Proposed refactor import { Accordion as AccordionBase } from './Accordion'
import { AccordionBody } from './AccordionBody'
import { AccordionHeader } from './AccordionHeader'
import { AccordionIndicator } from './AccordionIndicator'
+import type { AccordionComponent } from './Accordion.d'
-type AccordionComponent = typeof AccordionBase & {
- Body: typeof AccordionBody
- Header: typeof AccordionHeader
- Indicator: typeof AccordionIndicator
-}
-
const Accordion = AccordionBase as AccordionComponent
Accordion.Body = AccordionBody
Accordion.Header = AccordionHeader
Accordion.Indicator = AccordionIndicator📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||
| Accordion.Body = AccordionBody | ||||||||||||||||||||||||||||||||||||
| Accordion.Header = AccordionHeader | ||||||||||||||||||||||||||||||||||||
| Accordion.Indicator = AccordionIndicator | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @ts-nocheck | ||
| import AppMain from './AppMain' | ||
|
|
||
| export default AppMain |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @ts-nocheck | ||
| import { Card } from './Card' | ||
|
|
||
| export default Card |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @ts-nocheck | ||
| import { CardHeader } from './CardHeader' | ||
|
|
||
| export default CardHeader |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,16 +1,24 @@ | ||||||||||||||||||||||||||||||||||||||
| // @ts-nocheck | ||||||||||||||||||||||||||||||||||||||
| import React from 'react' | ||||||||||||||||||||||||||||||||||||||
| import classNames from 'classnames' | ||||||||||||||||||||||||||||||||||||||
| import { Input as RSCustomInput } from 'reactstrap' | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const CustomInput = (props) => { | ||||||||||||||||||||||||||||||||||||||
| const { className, ...otherProps } = props | ||||||||||||||||||||||||||||||||||||||
| type CustomInputProps = React.ComponentProps<typeof RSCustomInput> | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const CustomInput: React.FC<CustomInputProps> = (props) => { | ||||||||||||||||||||||||||||||||||||||
| const { className, label, name, ...otherProps } = props | ||||||||||||||||||||||||||||||||||||||
| const inputClass = classNames(className, { | ||||||||||||||||||||||||||||||||||||||
| 'custom-control-empty': !props.label, | ||||||||||||||||||||||||||||||||||||||
| 'custom-control-empty': !label, | ||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return <RSCustomInput data-testid={props.name} className={inputClass} {...otherProps} /> | ||||||||||||||||||||||||||||||||||||||
| return ( | ||||||||||||||||||||||||||||||||||||||
| <RSCustomInput | ||||||||||||||||||||||||||||||||||||||
| data-testid={name} | ||||||||||||||||||||||||||||||||||||||
| className={inputClass} | ||||||||||||||||||||||||||||||||||||||
| label={label} | ||||||||||||||||||||||||||||||||||||||
| name={name} | ||||||||||||||||||||||||||||||||||||||
| {...otherProps} | ||||||||||||||||||||||||||||||||||||||
| /> | ||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+13
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider handling undefined When 💡 Proposed fix return (
<RSCustomInput
- data-testid={name}
+ data-testid={name ?? undefined}
className={inputClass}
label={label}
name={name}
{...otherProps}
/>
)Or conditionally spread the attribute: return (
<RSCustomInput
{...(name && { 'data-testid': name })}
className={inputClass}
label={label}
name={name}
{...otherProps}
/>
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| CustomInput.propTypes = { ...RSCustomInput.propTypes } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| export { CustomInput } | ||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @ts-nocheck | ||
| import { CustomInput } from './CustomInput' | ||
|
|
||
| export default CustomInput |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| // @ts-nocheck | ||
| import { Divider } from './Divider' | ||
|
|
||
| export default Divider |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { makeStyles } from 'tss-react/mui' | ||
| import { SPACING } from '@/constants' | ||
|
|
||
| /** Min width per cell for ~5 columns on desktop; prevents label/value overlap */ | ||
| const MIN_CELL_WIDTH = 160 | ||
|
|
||
| export const useStyles = makeStyles()(() => ({ | ||
| detailGrid: { | ||
| display: 'grid', | ||
| gridTemplateColumns: `repeat(auto-fill, minmax(${MIN_CELL_WIDTH}px, 1fr))`, | ||
| gap: `${SPACING.SECTION_GAP}px ${SPACING.SECTION_GAP}px`, | ||
| width: '100%', | ||
| minWidth: 0, | ||
| }, | ||
| detailItem: { | ||
| minWidth: 0, | ||
| overflowWrap: 'break-word', | ||
| wordBreak: 'break-word', | ||
| }, | ||
| })) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| import React, { memo } from 'react' | ||
| import GluuFormDetailRow from 'Routes/Apps/Gluu/GluuFormDetailRow' | ||
| import { useStyles } from './GluuDetailGrid.style' | ||
| import type { GluuDetailGridProps, GluuDetailGridField } from './types' | ||
|
|
||
| const getFieldKey = (field: GluuDetailGridField, idx: number): string => | ||
| field.doc_entry ?? `${field.label}-${idx}` | ||
|
|
||
| const GluuDetailGrid: React.FC<GluuDetailGridProps> = ({ | ||
| fields, | ||
| labelStyle, | ||
| defaultDocCategory, | ||
| className, | ||
| layout = 'column', | ||
| }) => { | ||
| const { classes } = useStyles() | ||
|
|
||
| return ( | ||
| <div className={`${classes.detailGrid} ${className ?? ''}`.trim()}> | ||
| {fields.map((field, idx) => ( | ||
| <div key={getFieldKey(field, idx)} className={classes.detailItem}> | ||
| <GluuFormDetailRow | ||
| label={field.label} | ||
| value={field.value} | ||
| doc_entry={field.doc_entry} | ||
| doc_category={field.doc_category ?? defaultDocCategory} | ||
| isBadge={field.isBadge} | ||
| badgeBackgroundColor={field.badgeBackgroundColor} | ||
| badgeTextColor={field.badgeTextColor} | ||
| isDirect={field.isDirect} | ||
| lsize={field.lsize} | ||
| rsize={field.rsize} | ||
| labelStyle={ | ||
| labelStyle || field.labelStyle ? { ...labelStyle, ...field.labelStyle } : undefined | ||
| } | ||
| valueStyle={field.valueStyle} | ||
| rowClassName={field.rowClassName} | ||
| layout={field.layout ?? layout} | ||
| /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| export default memo(GluuDetailGrid) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import React from 'react' | ||
| import { render, screen } from '@testing-library/react' | ||
| import { GluuDetailGrid } from '@/components/GluuDetailGrid' | ||
| import AppTestWrapper from 'Routes/Apps/Gluu/Tests/Components/AppTestWrapper' | ||
|
|
||
| const Wrapper = ({ children }: { children: React.ReactNode }) => ( | ||
| <AppTestWrapper>{children}</AppTestWrapper> | ||
| ) | ||
|
|
||
| const mockFields = [ | ||
| { label: 'fields.inum', value: '09A0-93D7', doc_entry: 'inum' }, | ||
| { label: 'fields.name', value: 'smpp', doc_entry: 'name' }, | ||
| { label: 'fields.script_type', value: 'person_authentication', doc_entry: 'scriptType' }, | ||
| ] | ||
|
|
||
| it('renders all field labels and values', () => { | ||
| render(<GluuDetailGrid fields={mockFields} />, { wrapper: Wrapper }) | ||
| expect(screen.getByText(/Inum/i)).toBeInTheDocument() | ||
| expect(screen.getByText(/Name/i)).toBeInTheDocument() | ||
| expect(screen.getByText(/Script Type/i)).toBeInTheDocument() | ||
| expect(screen.getByText('09A0-93D7')).toBeInTheDocument() | ||
| expect(screen.getByText('smpp')).toBeInTheDocument() | ||
| expect(screen.getByText('person_authentication')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('renders with defaultDocCategory', () => { | ||
| render(<GluuDetailGrid fields={mockFields} defaultDocCategory="SCRIPT" />, { | ||
| wrapper: Wrapper, | ||
| }) | ||
| expect(screen.getByText('smpp')).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('renders with labelStyle', () => { | ||
| render(<GluuDetailGrid fields={mockFields} labelStyle={{ color: 'red' }} />, { wrapper: Wrapper }) | ||
| expect(screen.getByText(/Inum/i)).toBeInTheDocument() | ||
| }) | ||
|
Comment on lines
+16
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider wrapping tests in a The tests provide basic validation but could be improved:
♻️ Suggested structure+describe('GluuDetailGrid', () => {
-it('renders all field labels and values', () => {
+ it('renders all field labels and values', () => {
render(<GluuDetailGrid fields={mockFields} />, { wrapper: Wrapper })
// ... assertions
-})
+ })
-it('renders with defaultDocCategory', () => {
+ it('renders with defaultDocCategory', () => {
// ...
-})
+ })
-it('renders with labelStyle', () => {
+ it('renders with labelStyle', () => {
// ...
+ })
+
+ it('handles empty fields array', () => {
+ render(<GluuDetailGrid fields={[]} />, { wrapper: Wrapper })
+ // Assert graceful empty state
+ })
})🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export { default as GluuDetailGrid } from './GluuDetailGrid' | ||
| export type { GluuDetailGridProps, GluuDetailGridField } from './types' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import type { CSSProperties } from 'react' | ||
|
|
||
| export type GluuDetailGridField = { | ||
| label: string | ||
| value?: string | number | boolean | null | ||
| doc_entry?: string | ||
| doc_category?: string | ||
| isBadge?: boolean | ||
| badgeBackgroundColor?: string | ||
| badgeTextColor?: string | ||
| isDirect?: boolean | ||
| lsize?: number | ||
| rsize?: number | ||
| labelStyle?: CSSProperties | ||
| valueStyle?: CSSProperties | ||
| rowClassName?: string | ||
| layout?: 'row' | 'column' | ||
| } | ||
|
|
||
| export type GluuDetailGridProps = { | ||
| fields: GluuDetailGridField[] | ||
| labelStyle?: CSSProperties | ||
| defaultDocCategory?: string | ||
| className?: string | ||
| layout?: 'row' | 'column' | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 2065
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 1720
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 506
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 368
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 1767
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 64
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 4274
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 2564
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 3860
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 2993
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 140
Remove the global
act()warning suppression.Suppressing React's "not wrapped in act()" warnings masks real async test defects. Tests should be fixed to properly handle state updates with
waitFor(),userEvent, or other React Testing Library patterns rather than hiding the warning.Suggested fix
- msg.includes('not wrapped in act(') ||📝 Committable suggestion
🤖 Prompt for AI Agents