-
Notifications
You must be signed in to change notification settings - Fork 164
Auto Internal Name: Filtering fields to show only short texts + refactor [MAPS-95] #10260
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
base: auto-internal-name
Are you sure you want to change the base?
Conversation
# Conflicts: # apps/auto-internal-name/src/components/OverrideRow.tsx # apps/auto-internal-name/src/locations/ConfigScreen.tsx # apps/auto-internal-name/test/components/OverrideRow.spec.tsx # apps/auto-internal-name/test/locations/ConfigScreen.spec.tsx # apps/auto-internal-name/test/mocks/mockSdk.ts
33f26ca to
4b220b6
Compare
# Conflicts: # apps/auto-internal-name/eslint.config.mts # apps/auto-internal-name/src/components/OverrideRow.tsx # apps/auto-internal-name/src/locations/ConfigScreen.tsx # apps/auto-internal-name/src/utils/types.ts # apps/auto-internal-name/test/components/OverrideRow.spec.tsx # apps/auto-internal-name/test/locations/ConfigScreen.spec.tsx # apps/auto-internal-name/test/mocks/mockSdk.ts
JuliRossi
left a comment
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.
⭐ Good on taking this refactor. Just left a few comments
| 'react/jsx-uses-react': 'off', | ||
| 'react/react-in-jsx-scope': 'off', | ||
| 'no-unused-vars': 'off', | ||
| 'react/prop-types': 'off', |
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.
❓ Why are we removing this restriction ?
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.
Oh yeah, that was because eslint wasn't letting me to infer the prop types, but I specified them and added the restriction again. ✅ Nice catch!
Purpose
The app works for short text fields only, so the single line editor is rendered in the Field location.
If source field id selected in the config screen is not from a short text, the value couldn’t be displayed correctly. The same happens when we add an override, so we should add this check/filter there too.
In addition, in this PR a pending refactor to extract a new component which handles all the actions for the overrides was added. Actions such as:
Approach
Filter the sourceFieldId and fieldName autocompletes to show only short text fields.
Testing steps
Added automated tests. Manual test:
Grabacion.de.pantalla.2025-11-18.a.la.s.12.14.47.p.m.mov
Breaking Changes
N/A
Dependencies and/or References
Link to MAPS-95
Deployment
N/A