feat(example-form): update example form #17028
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (22)
WalkthroughThis pull request introduces comprehensive modifications across multiple libraries and templates, focusing on enhancing form-building capabilities, field configurations, and application template structures. The changes span various aspects of the application, including field builders, form templates, data schemas, and utility functions. Key modifications include updates to field builder functions, introduction of new components, refinement of type definitions, and improvements in state machine configurations. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form
participant FieldBuilder
participant StateMachine
User->>Form: Starts Application
Form->>FieldBuilder: Request Field Configuration
FieldBuilder-->>Form: Return Configured Fields
Form->>StateMachine: Determine Current State
StateMachine-->>Form: Provide Appropriate Form
User->>Form: Fills Out Form
Form->>StateMachine: Submit Application
StateMachine->>StateMachine: Update Application State
Possibly Related PRs
Suggested Labels
Suggested Reviewers
Tip You can disable the changed files summary in the walkthrough.Disable the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportBranch report: ✅ 0 Failed, 4 Passed, 0 Skipped, 2.73s Total Time |
jonnigs
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (50)
libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/fileUploadSubsection.ts (1)
6-15: 🛠️ Refactor suggestion
Enhance the file upload configuration for better reusability and user experience.
The file upload configuration could benefit from additional configuration options to make it more robust and reusable across different use cases.
Consider applying these improvements:
export const fileUploadSubsection = buildSubSection({ id: 'fileUpload', - title: 'File Upload', + title: { defaultMessage: 'File Upload', id: 'fileUpload.title' }, children: [ buildFileUploadField({ id: 'fileUpload', - title: 'File Upload', + title: { defaultMessage: 'File Upload', id: 'fileUpload.field.title' }, + maxSize: 10 * 1024 * 1024, // 10MB + accept: ['.pdf', '.doc', '.docx'], + required: true, + description: { + defaultMessage: 'Upload relevant documents in PDF or Word format (max 10MB)', + id: 'fileUpload.field.description' + }, }), ], })The suggested improvements:
- Add internationalization support for text strings
- Configure file size limits and accepted file types
- Add field requirements and description
- Make the configuration more self-documenting
Committable suggestion skipped: line range outside the PR's diff.
libs/application/ui-fields/src/lib/SliderFormField/SliderFormField.tsx (1)
12-12: 🛠️ Refactor suggestion
Remove unused import
The
Textcomponent is imported but not used in the code. Removing unused imports helps with tree-shaking optimization.-import { Text, Box } from '@island.is/island-ui/core' +import { Box } from '@island.is/island-ui/core'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { Box } from '@island.is/island-ui/core'libs/application/ui-fields/src/lib/FieldsRepeaterFormField/FieldsRepeaterItem.tsx (1)
219-219: 🛠️ Refactor suggestion
Add type definition and make the
largeprop configurable.The hardcoded
large={true}prop could limit component reusability. Consider:
- Adding the prop to the
ItemFieldPropsinterface- Making it configurable through the component's props
Apply these changes:
interface ItemFieldProps { application: Application error?: string item: RepeaterItem & { id: string } dataId: string index: number values: Array<Record<string, string>> + large?: boolean // Optional to maintain backward compatibility }Then use it in the component:
-large={true} +large={props.large}Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/fields/README.md (1)
12-21: 🛠️ Refactor suggestion
Fix typos and enhance usage guidelines with technical considerations.
There are several issues to address:
- "Useage" should be "Usage"
- "built in" should be hyphenated as "built-in"
- "can not" should be "cannot"
- "fullfill" should be "fulfill"
Additionally, consider adding technical guidelines for optimal bundling.
-## Useage of custom components +## Usage of custom components Before creating a custom component, you should: -1. Try to use the shared components, `buildTextField`, `buildCheckboxField`, `buildSelectField`, `buildFileUploadField` and so on. This is most preferable to make the look and feel of the application more consistent and uniform. -2. If the shared components almost fullfill your needs but you need something more, consider consulting with the designer of the application and try to adjust the design to the built in components. -3. If the design can not be adjusted to the built in components, then consult Norda if a shared component can possibly be adjusted or expanded to fulfill your needs. +1. Try to use the shared components, `buildTextField`, `buildCheckboxField`, `buildSelectField`, `buildFileUploadField` and so on. This is most preferable to make the look and feel of the application more consistent and uniform. +2. If the shared components almost fulfill your needs but you need something more, consider consulting the designer of the application and try to adjust the design to the built-in components. +3. If the design cannot be adjusted to the built-in components, then consult Norda if a shared component can possibly be adjusted or expanded to fulfill your needs. 4. Is there another application that has made a similar custom component before? If so, then it should be a shared component. 5. If you still need a new component, ask yourself if this is a component that another application might also need in the future. If so make the new component shared. 6. Make a custom component if none of the above apply. + +Technical considerations: +- Ensure components are tree-shakeable by using named exports +- Keep dependencies minimal to optimize bundle size +- Consider lazy loading for larger components📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.## Usage of custom components Before creating a custom component, you should: 1. Try to use the shared components, `buildTextField`, `buildCheckboxField`, `buildSelectField`, `buildFileUploadField` and so on. This is most preferable to make the look and feel of the application more consistent and uniform. 2. If the shared components almost fulfill your needs but you need something more, consider consulting the designer of the application and try to adjust the design to the built-in components. 3. If the design cannot be adjusted to the built-in components, then consult Norda if a shared component can possibly be adjusted or expanded to fulfill your needs. 4. Is there another application that has made a similar custom component before? If so, then it should be a shared component. 5. If you still need a new component, ask yourself if this is a component that another application might also need in the future. If so make the new component shared. 6. Make a custom component if none of the above apply. Technical considerations: - Ensure components are tree-shakeable by using named exports - Keep dependencies minimal to optimize bundle size - Consider lazy loading for larger components🧰 Tools
🪛 LanguageTool
[style] ~17-~17: This phrase is redundant. Consider writing “consulting”.
Context: ...s but you need something more, consider consulting with the designer of the application and try...(CONSULT_WITH)
[grammar] ~17-~17: A hyphen is missing in the adjective “built-in”.
Context: ...ion and try to adjust the design to the built in components. 3. If the design can not be...(BUILT_IN_HYPHEN)
[style] ~18-~18: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e built in components. 3. If the design can not be adjusted to the built in components,...(CAN_NOT_PREMIUM)
[grammar] ~18-~18: A hyphen is missing in the adjective “built-in”.
Context: ...f the design can not be adjusted to the built in components, then consult Norda if a sha...(BUILT_IN_HYPHEN)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...lication might also need in the future. If so make the new component shared. 6. Ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
libs/application/templates/reference-template/src/lib/dataSchema.ts (1)
35-35:
⚠️ Potential issueReplace 'isNaN' with 'Number.isNaN' for accurate NaN checking
The global
isNaNfunction performs type coercion, which can lead to unexpected results. UsingNumber.isNaNensures that the check only returnstruewhen the value is actuallyNaN.Apply this diff to fix the issue:
- if (isNaN(asNumber)) { + if (Number.isNaN(asNumber)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if (Number.isNaN(asNumber)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 35-35: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
libs/application/templates/reference-template/src/forms/exampleForm/tablesAndRepeatersSection/tableRepeaterSubsection.ts (2)
45-66:
⚠️ Potential issueAlign static data values with defined option values
The
select,radio, andcheckboxfields in your static table data use values like'option 1', whereas the option values are'option1'without the space. This mismatch can cause issues in data binding and display.Apply this diff to correct the static data:
- select: 'option 1', + select: 'option1', - radio: 'option 2', + radio: 'option2', - checkbox: 'option 3', + checkbox: 'option3',Repeat these corrections for all relevant entries in the static data.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.return [ { input: 'John Doe', select: 'option1', radio: 'option2', checkbox: 'option3', date: '2024-01-01', name: 'Test Name', nationalId: '000000-0000', phone: '6666666', }, { input: 'Jane Doe', select: 'option1', radio: 'option2', checkbox: 'option3', date: '2024-01-01', name: 'Test Name 2', nationalId: '100000-0000', phone: '6666666', }, ]
8-9: 🛠️ Refactor suggestion
Avoid deep imports to improve maintainability and bundling
Importing modules directly from deep paths within a library can lead to issues with module resolution and bundling. It's recommended to import from the package's public API.
Apply this diff to fix the imports:
- import { input } from 'libs/island-ui/core/src/lib/Input/Input.mixins' - import { name } from 'libs/island-ui/core/src/lib/FormStepper/FormStepperSection.css' + import { Input } from '@island.is/island-ui/core'Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/parental-leave/src/forms/ParentalLeaveForm.ts (1)
1228-1228:
⚠️ Potential issueCorrect 'gridTemplateRows' in 'trackStyle' to a valid CSS string
The
gridTemplateRowsproperty should be a string representing a valid CSS grid template. Setting it to a number may not work as intended.Apply this diff to correct the
gridTemplateRowsvalue:- trackStyle: { gridTemplateRows: 8 }, + trackStyle: { gridTemplateRows: 'repeat(8, 1fr)' },Also applies to: 1323-1323, 1372-1372
libs/application/templates/reference-template/src/graphql/sampleQuery.ts (1)
2-18: 🛠️ Refactor suggestion
Add TypeScript type definitions for the query response.
While the query is well-structured, it lacks TypeScript type definitions which are crucial for type safety across the application.
interface School { id: string nationalId: string name: string type: string children: Array<{ id: string nationalId: string name: string type: string gradeLevels: string[] }> } interface FriggSchoolsByMunicipalityResponse { friggSchoolsByMunicipality: School[] }Also, consider implementing field-level pagination for the children array if the dataset could be large, as fetching all children data upfront might impact performance.
libs/application/templates/reference-template/src/forms/exampleForm/compositeFieldsSection/applicantInfoSubsection.ts (1)
5-6:
⚠️ Potential issueFix typos in 'applicant' spelling
There are spelling errors in the id and title:
- id: 'aplicantInfoSubsection', - title: 'Aplicant Info Subsection', + id: 'applicantInfoSubsection', + title: 'Applicant Info Subsection',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.id: 'applicantInfoSubsection', title: 'Applicant Info Subsection',libs/application/templates/reference-template/src/forms/exampleForm/compositeFieldsSection/nationalIdWithNameSubsection.ts (1)
11-12: 💡 Codebase verification
ID 'pickRole.electPerson' should be renamed to match reference template context
The ID appears to be copied from the announcement-of-death template where it's used in a specific context for delegating roles. In the reference template, this ID should be generic and follow the template's naming convention.
- Change
id: 'pickRole.electPerson'toid: 'nationalIdLookup'or similar generic name that reflects its purpose in the reference template- Consider removing the nested path structure (dot notation) since this is a standalone example
🔗 Analysis chain
Verify the context of the field ID
The ID 'pickRole.electPerson' seems specific to a particular use case rather than a generic reference template example.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of this ID to understand its context rg "pickRole.electPerson" --type tsLength of output: 317
Script:
#!/bin/bash # Let's check the content of both files to understand the context better echo "=== Reference template implementation ===" cat libs/application/templates/reference-template/src/forms/exampleForm/compositeFieldsSection/nationalIdWithNameSubsection.ts echo -e "\n=== Announcement of death implementation ===" cat libs/application/templates/announcement-of-death/src/forms/prerequisite/index.tsLength of output: 8839
libs/application/templates/reference-template/src/forms/exampleForm/commonActionsSection/index.ts (1)
1-6:
⚠️ Potential issueFix typo in import path name.
There appears to be a typo in the import path: 'validadionSubsection' should be 'validationSubsection'.
-import { validationSubsection } from './validadionSubsection' +import { validationSubsection } from './validationSubsection'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.import { buildSection } from '@island.is/application/core' import { conditionsSubsection } from './conditionsSubsection' import { conditions2Subsection } from './conditions2Subsection' import { getDataFromExternalDataSubsection } from './getDataFromExternalDataSection' import { validationSubsection } from './validationSubsection'libs/application/templates/reference-template/src/utils/constants.ts (1)
15-20: 🛠️ Refactor suggestion
Consider making career history options more generic
The
CareerHistoryEnumcontains specific company names (ARANJA, ADVANIA) which might not be suitable for a reusable reference template.export enum CareerHistoryEnum { GOVERNMENT = 'government', - ARANJA = 'aranja', - ADVANIA = 'advania', + PRIVATE_SECTOR = 'privateSector', + CONTRACTOR = 'contractor', OTHER = 'other', }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export enum CareerHistoryEnum { GOVERNMENT = 'government', PRIVATE_SECTOR = 'privateSector', CONTRACTOR = 'contractor', OTHER = 'other', }libs/application/templates/reference-template/src/forms/exampleForm/commonActionsSection/conditions2Subsection.ts (1)
9-16: 🛠️ Refactor suggestion
Simplify condition logic and add type safety
The condition function could be simplified and made more type-safe.
- condition: (answers) => { - const checkbox2Value = getValueViaPath<Array<string>>( - answers, - 'conditions2Checkbox', - ) - - return checkbox2Value ? checkbox2Value[0] === YES : false - }, + condition: (answers: FormValue) => { + const checkbox2Value = getValueViaPath<string[]>(answers, 'conditions2Checkbox') + return checkbox2Value?.[0] === YES + },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.condition: (answers: FormValue) => { const checkbox2Value = getValueViaPath<string[]>(answers, 'conditions2Checkbox') return checkbox2Value?.[0] === YES },libs/application/templates/reference-template/src/components/Logo/Logo.tsx (1)
12-29: 🛠️ Refactor suggestion
Multiple improvements needed for robustness
- Extract magic strings to constants
- Add error handling for dynamic imports
- Consider caching logos to prevent unnecessary reloads
+const SPECIAL_APPLICANT_ID = '0101302399' +const TOWN_LOGOS = { + DEFAULT: 'sambandid', + SPECIAL: 'isafjardarbaer', + REGULAR: 'akureyri' +} as const export const Logo = ({ application }: Props) => { const [logo, setLogo] = useState<string>() + const [error, setError] = useState<string>() useEffect(() => { const getLogo = async () => { + try { const applicant = application.applicant const town = !applicant - ? 'sambandid' - : applicant === '0101302399' - ? 'isafjardarbaer' - : 'akureyri' + ? TOWN_LOGOS.DEFAULT + : applicant === SPECIAL_APPLICANT_ID + ? TOWN_LOGOS.SPECIAL + : TOWN_LOGOS.REGULAR const svgLogo = await import(`../../assets/${town}.svg`) setLogo(svgLogo.default) + } catch (err) { + setError('Failed to load logo') + console.error('Logo loading error:', err) + } } getLogo() }, []) - return <img src={logo} alt="Municipality logo" /> + if (error) return <div>{error}</div> + return logo ? <img src={logo} alt="Municipality logo" /> : null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const SPECIAL_APPLICANT_ID = '0101302399' const TOWN_LOGOS = { DEFAULT: 'sambandid', SPECIAL: 'isafjardarbaer', REGULAR: 'akureyri' } as const export const Logo = ({ application }: Props) => { const [logo, setLogo] = useState<string>() const [error, setError] = useState<string>() useEffect(() => { const getLogo = async () => { try { const applicant = application.applicant const town = !applicant ? TOWN_LOGOS.DEFAULT : applicant === SPECIAL_APPLICANT_ID ? TOWN_LOGOS.SPECIAL : TOWN_LOGOS.REGULAR const svgLogo = await import(`../../assets/${town}.svg`) setLogo(svgLogo.default) } catch (err) { setError('Failed to load logo') console.error('Logo loading error:', err) } } getLogo() }, []) if (error) return <div>{error}</div> return logo ? <img src={logo} alt="Municipality logo" /> : nulllibs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/companySearchSubsection.ts (1)
7-33: 🛠️ Refactor suggestion
Enhance maintainability and internationalization
Consider the following improvements:
- Use more specific IDs to prevent collisions
- Extract titles to translation files
- Reduce duplication in company search fields
+const COMPANY_SEARCH_IDS = { + SUBSECTION: 'referenceTemplate.companySearch.subsection', + MULTIFIELD: 'referenceTemplate.companySearch.multifield', + BASIC: 'referenceTemplate.companySearch.basic', + ISAT: 'referenceTemplate.companySearch.isat', + FORBIDDEN: 'referenceTemplate.companySearch.forbidden', +} as const export const companySearchSubsection = buildSubSection({ - id: 'companySearchSubsection', + id: COMPANY_SEARCH_IDS.SUBSECTION, - title: 'Company Search Subsection', + title: { id: 'company.search.subsection.title' }, children: [ buildMultiField({ - id: 'companySearchMultiField', + id: COMPANY_SEARCH_IDS.MULTIFIELD, - title: 'Company Search MultiField', + title: { id: 'company.search.multifield.title' }, children: [ buildCompanySearchField({ - id: 'companySearch', + id: COMPANY_SEARCH_IDS.BASIC, - title: 'Company Search', + title: { id: 'company.search.basic.title' }, placeholder: 'Search for a company', }), // ... similar changes for other fieldsCommittable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/fields/TestCustomComponent/TestCustomComponent.tsx (1)
33-37: 🛠️ Refactor suggestion
Improve accessibility and layout handling
The current implementation has several areas for improvement:
- Missing semantic HTML and ARIA attributes
- Potential layout issues with space-between when items wrap
- No handling for long text content
- <Box padding={2} display="flex" justifyContent="spaceBetween"> - {someData.map((item) => ( - <p key={item}>{item}</p> - ))} + <Box padding={2}> + <ul role="list" className={styles.itemList}> + {someData.map((item) => ( + <li key={item} className={styles.item}> + <Text truncate>{item}</Text> + </li> + ))} + </ul> </Box>Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/exampleForm/overviewSection/overviewSection.ts (1)
24-28: 🛠️ Refactor suggestion
Enhance type safety for custom component reference
The component name is currently a string literal, which could lead to runtime errors if mistyped.
+type CustomComponents = 'Overview' | 'TestCustomComponent'; + buildCustomField({ id: 'customComponent', title: '', - component: 'Overview', + component: 'Overview' as CustomComponents, }),Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/exampleForm/customSection/customSection.ts (1)
29-38: 🛠️ Refactor suggestion
Add type safety for custom component data
The
someDataproperty lacks type definition. Consider creating an interface for the custom component's data props.interface TestCustomComponentData { someData: string[]; } buildCustomField<TestCustomComponentData>( { id: 'customComponent', title: 'The custom component', component: 'TestCustomComponent', }, { someData: ['foo', 'bar', 'baz'], }, )libs/application/templates/reference-template/src/forms/exampleForm/tablesAndRepeatersSection/StaticTableSubsection.ts (2)
19-38: 🛠️ Refactor suggestion
Add type safety for table data and consider data structure
The table data is hardcoded and lacks type safety. Consider:
- Creating interfaces for row data
- Using objects instead of arrays for better maintainability
interface TableRow { column1: string; column2: string; column3: string; column4: string; } const rows: TableRow[] = [ { column1: 'Row 1, Column 1', column2: 'Row 1, Column 2', column3: 'Row 1, Column 3', column4: 'Row 1, Column 4', }, // ... other rows ]; // Transform to array format for the table const tableRows = rows.map(row => Object.values(row));
13-18: 🛠️ Refactor suggestion
Add type safety and localization for table headers
The table headers are hardcoded strings. Consider:
- Using the message system for internationalization
- Adding type safety with an enum or const object
enum TableColumns { Column1 = 'column1', Column2 = 'column2', Column3 = 'column3', Column4 = 'column4', } const header = Object.values(TableColumns).map(column => m[`table.header.${column}`]);libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/accordionSubsection.ts (1)
13-24: 🛠️ Refactor suggestion
Enhance accordion items with meaningful content and accessibility attributes.
The current Lorem ipsum content should be replaced with meaningful example content. Additionally, consider adding accessibility attributes for better screen reader support.
accordionItems: [ { itemTitle: 'Item 1', + ariaLabel: 'Example of form section 1', itemContent: - 'Lorem ipsum dolor sit amet, consectetur adipiscing elit...', + 'This section demonstrates how to structure a basic form segment. It includes examples of field validation and user input handling.', }, { itemTitle: 'Item 2', + ariaLabel: 'Example of form section 2', itemContent: - 'Lorem ipsum dolor sit amet, consectetur adipiscing elit...', + 'This section shows advanced form features like conditional rendering and dynamic field updates based on user input.', }, ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.accordionItems: [ { itemTitle: 'Item 1', ariaLabel: 'Example of form section 1', itemContent: 'This section demonstrates how to structure a basic form segment. It includes examples of field validation and user input handling.', }, { itemTitle: 'Item 2', ariaLabel: 'Example of form section 2', itemContent: 'This section shows advanced form features like conditional rendering and dynamic field updates based on user input.', }, ],libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/keyValueSubsection.ts (1)
25-25:
⚠️ Potential issueFix typo in the label property.
There's a typo in "displax" which should be "display".
- label: 'Key value displax flex', + label: 'Key value display flex',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.label: 'Key value display flex',libs/application/templates/reference-template/src/utils/options.ts (1)
1-4: 🛠️ Refactor suggestion
Add TypeScript interfaces and documentation.
Enhance type safety and maintainability by adding TypeScript interfaces and JSDoc comments.
+/** Type for basic form option structure */ +interface FormOption { + label: string; + value: string; +} + +/** Type for radio options with illustrations */ +interface RadioIllustrationOption extends FormOption { + illustration: any; // Consider using a more specific type +} + import { plate110 } from '../assets/plate-110-510' import { plate200 } from '../assets/plate-200-280' import { plate155 } from '../assets/plate-155-305'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements./** Type for basic form option structure */ interface FormOption { label: string; value: string; } /** Type for radio options with illustrations */ interface RadioIllustrationOption extends FormOption { illustration: any; // Consider using a more specific type } import { plate110 } from '../assets/plate-110-510' import { plate200 } from '../assets/plate-200-280' import { plate155 } from '../assets/plate-155-305'libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/selectSubsection.ts (1)
7-7:
⚠️ Potential issueRemove unused import
The
titleimport from 'process' is not used in this file.-import { title } from 'process'libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/radioSubsection.ts (1)
51-55: 🛠️ Refactor suggestion
Inconsistent ID casing in field identifier
The ID 'WhiteBackgroundRadio' uses PascalCase while other IDs use camelCase. This inconsistency could cause issues with form state management.
buildRadioField({ - id: 'WhiteBackgroundRadio', + id: 'whiteBackgroundRadio', title: 'White background (try to use blue if possible)', options: radioOptions, backgroundColor: 'white', }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.id: 'whiteBackgroundRadio', title: 'White background (try to use blue if possible)', options: radioOptions, backgroundColor: 'white', }),libs/application/templates/reference-template/src/forms/exampleForm/commonActionsSection/validadionSubsection.ts (3)
1-1:
⚠️ Potential issueFix typo in filename
The filename contains a typo: 'validadionSubsection.ts' should be 'validationSubsection.ts'
Please rename the file to fix the typo.
28-30: 🛠️ Refactor suggestion
Use message system for internationalization
The description text is hardcoded in Icelandic. Consider using the message system (m) for consistency and internationalization support.
description: - 'Allir reitir á þessari síðu eru með validation sem verður að fylla út til að halda áfram', + m.validationRequiredFields,Committable suggestion skipped: line range outside the PR's diff.
31-35: 🛠️ Refactor suggestion
Add explicit validation rules for text field
The text field requires 3 or more characters, but this validation rule isn't explicitly defined in the field configuration.
buildTextField({ id: 'validation.validationTextField', title: 'Verður að vera 3 stafir eða meira', required: true, // Adds the red star to the field + minLength: 3, + validate: (value) => value.length >= 3 ? true : 'Text must be at least 3 characters long', }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.buildTextField({ id: 'validation.validationTextField', title: 'Verður að vera 3 stafir eða meira', required: true, // Adds the red star to the field minLength: 3, validate: (value) => value.length >= 3 ? true : 'Text must be at least 3 characters long', }),libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/actionCardSubsection.ts (1)
20-25: 🛠️ Refactor suggestion
Move external URLs to configuration
Hardcoded URLs should be moved to a configuration file for better maintainability and security validation.
Consider creating a configuration file for external URLs and importing them:
// config/externalUrls.ts export const externalUrls = { googleSearch: 'https://www.google.com', // add other URLs here } as const;Then update the code:
cta: { label: 'Card cta', variant: 'primary', external: true, - to: 'https://www.google.com', + to: externalUrls.googleSearch, },libs/application/ui-fields/src/lib/SliderFormField/SliderFormField.stories.mdx (1)
39-39: 💡 Codebase verification
titleis not a required field in the componentLooking at the code:
- The
titleprop is used in the stories but not in the actual component implementation- The
SliderFormFieldPropsinterface only requiresfield: SliderFieldandapplication: Application- The component's implementation doesn't use the
titleprop anywhere in its render logicThe documentation should be updated to:
- Remove
titlefrom the usage example since it's not a supported prop- Update the ArgsTable to reflect the actual props
🔗 Analysis chain
Verify if
titleis a required field and update documentation accordinglyThe addition of the
titleproperty in both the usage example and story implementation is consistent. However, we should ensure that:
- The documentation reflects whether this is a required field
- The ArgsTable at the bottom of the file shows this new property
Also applies to: 66-66
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if title is marked as required in the component props ast-grep --pattern 'interface $interface { $$$ title: string $$$ }'Length of output: 75
Script:
#!/bin/bash # Let's check the component file for props interface and title prop fd SliderFormField.tsx -x cat {} # Also check the stories file to see if title is documented fd SliderFormField.stories.mdx -x cat {}Length of output: 5324
libs/application/templates/reference-template/src/forms/exampleForm/commonActionsSection/getDataFromExternalDataSection.ts (1)
27-33: 🛠️ Refactor suggestion
Enhance error handling for external data retrieval
While the null coalescing operator handles missing values, consider adding type guards and error boundaries for more robust error handling when accessing deeply nested external data.
- const value = getValueViaPath<string>( - application.externalData, - 'getReferenceData.data.referenceData.applicantName', - ) - - return value ?? 'Fannst ekki' + try { + const value = getValueViaPath<string>( + application.externalData, + 'getReferenceData.data.referenceData.applicantName', + ) + return value ?? 'Fannst ekki' + } catch (error) { + console.error('Error retrieving applicant name:', error) + return 'Villa kom upp' + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.try { const value = getValueViaPath<string>( application.externalData, 'getReferenceData.data.referenceData.applicantName', ) return value ?? 'Fannst ekki' } catch (error) { console.error('Error retrieving applicant name:', error) return 'Villa kom upp' }libs/application/templates/reference-template/src/fields/Overview/Overview.tsx (1)
51-60: 🛠️ Refactor suggestion
Improve array rendering and type safety
- Using array index as key is an anti-pattern in React
- The static analysis tool correctly suggests using optional chaining
- {tableRepeaterAnswers && - tableRepeaterAnswers.map((ans, i) => { - return ( - <KeyValue - key={i} - label={`Value ${i + 1}`} - value={`${ans.fullName} - ${ans.nationalId} - ${ans.relation}`} - /> - ) - })} + {tableRepeaterAnswers?.map((ans) => ( + <KeyValue + key={`${ans.nationalId}-${ans.fullName}`} + label={ans.relation} + value={`${ans.fullName} - ${ans.nationalId}`} + /> + ))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.{tableRepeaterAnswers?.map((ans) => ( <KeyValue key={`${ans.nationalId}-${ans.fullName}`} label={ans.relation} value={`${ans.fullName} - ${ans.nationalId}`} /> ))}🧰 Tools
🪛 Biome (1.9.4)
[error] 51-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/descriptionSubsection.ts (1)
16-21:
⚠️ Potential issueFix duplicate field IDs
Multiple description fields share the same ID 'description', which could cause issues with field identification and state management. Each field should have a unique identifier.
Apply this pattern to make IDs unique:
- id: 'description', + id: 'descriptionDefault',- id: 'description', + id: 'descriptionH1',And so on for each description field.
Also applies to: 22-27, 29-36, 37-43, 44-50, 51-57, 58-64, 65-74, 75-80
libs/application/templates/reference-template/src/forms/exampleForm/tablesAndRepeatersSection/fieldsRepeaterSubsection.ts (1)
19-21: 🛠️ Refactor suggestion
Move hardcoded strings to message constants
The description text is hardcoded in a non-English language. For better internationalization support, move it to the messages file.
- description: - 'FieldsRepeater virkar svipað og tableRepeater, að því leiti að inni í honum er skilgreint eitthvað sett af field-um til að fylla út í og þetta sett er hægt að endurtaka eins oft og þarf. Munurinn er að í tableRepeater fara gildin inn í töflu en í fieldsRepeater þá sjást alltaf öll field sem búið er að búa til.', + description: m.fieldsRepeaterDescription,Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/prerequisitesForm/prerequisitesForm.ts (1)
26-26: 🛠️ Refactor suggestion
Move hardcoded strings to message constants
Several strings are hardcoded in mixed languages. For better internationalization support, move them to the messages file.
- title: 'Forkröfur', + title: m.prerequisitesTitle,- title: 'Þjóðskrá', - subTitle: 'Upplýsingar um þig í Þjóðskrá.', + title: m.nationalRegistryTitle, + subTitle: m.nationalRegistrySubtitle,Also applies to: 42-42, 56-57, 61-62
libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/asyncSelectSubsection.ts (2)
23-35: 🛠️ Refactor suggestion
Refactor duplicate GraphQL query logic
The
loadOptionsfunction is duplicated across three async select fields. This violates the DRY principle and makes maintenance harder.Extract the common logic into a shared function:
+ const loadMunicipalityOptions = async (apolloClient) => { + const { data } = await apolloClient.query<FriggSchoolsByMunicipality>({ + query: friggSchoolsByMunicipalityQuery, + }) + + return data?.friggSchoolsByMunicipality?.map((municipality) => ({ + value: municipality.name, + label: municipality.name, + })) ?? [] + } buildAsyncSelectField({ id: 'asyncSelect', title: 'Async Select', placeholder: 'Placeholder text', loadingError: 'Loading error', - loadOptions: async ({ apolloClient }) => { - const { data } = await apolloClient.query<FriggSchoolsByMunicipality>({ - query: friggSchoolsByMunicipalityQuery, - }) - return ( - data?.friggSchoolsByMunicipality?.map((municipality) => ({ - value: municipality.name, - label: municipality.name, - })) ?? [] - ) - }, + loadOptions: ({ apolloClient }) => loadMunicipalityOptions(apolloClient), }),Also applies to: 42-54, 61-73
23-35:
⚠️ Potential issueAdd error handling for GraphQL query failures
The GraphQL query could fail but there's no error handling in place.
Add try-catch block:
loadOptions: async ({ apolloClient }) => { + try { const { data } = await apolloClient.query<FriggSchoolsByMunicipality>({ query: friggSchoolsByMunicipalityQuery, }) return ( data?.friggSchoolsByMunicipality?.map((municipality) => ({ value: municipality.name, label: municipality.name, })) ?? [] ) + } catch (error) { + console.error('Failed to load municipality options:', error) + return [] + } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.loadOptions: async ({ apolloClient }) => { try { const { data } = await apolloClient.query<FriggSchoolsByMunicipality>({ query: friggSchoolsByMunicipalityQuery, }) return ( data?.friggSchoolsByMunicipality?.map((municipality) => ({ value: municipality.name, label: municipality.name, })) ?? [] ) } catch (error) { console.error('Failed to load municipality options:', error) return [] } },libs/application/templates/reference-template/src/forms/exampleForm/commonActionsSection/conditionsSubsection.ts (2)
52-59: 🛠️ Refactor suggestion
Simplify condition logic and add type safety
The condition logic is duplicated and lacks type safety for the answers parameter.
Extract and type the condition logic:
+ const isCheckboxChecked = (answers: FormValue, checkboxId: string): boolean => { + const checkboxValue = getValueViaPath<Array<string>>(answers, checkboxId) + return checkboxValue?.[0] === YES + } buildTextField({ - condition: (answers) => { - const checkboxValue = getValueViaPath<Array<string>>( - answers, - 'conditionsCheckbox', - ) - return checkboxValue ? checkboxValue[0] === YES : false - }, + condition: (answers) => isCheckboxChecked(answers, 'conditionsCheckbox'), id: 'conditionsTextField',Also applies to: 79-86
87-87:
⚠️ Potential issueFix duplicate field ID
The ID 'conditionsTextField' is used twice (lines 61 and 87), which could cause conflicts.
Change the ID of the hidden input:
- id: 'conditionsTextField', + id: 'conditions2HiddenField',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.id: 'conditions2HiddenField',libs/application/templates/reference-template/src/forms/exampleForm/index.ts (2)
73-81: 🛠️ Refactor suggestion
Add type safety for external data access
Accessing externalData without type checking could lead to runtime errors.
Add type checking:
+ type ApplicationResult = { + data: { id: string } + } const sendApplicationActionResult = - application.externalData[ApiActions.createApplication] + application.externalData[ApiActions.createApplication] as ApplicationResult | undefined - let id = 'unknown' - if (sendApplicationActionResult) { - // @ts-ignore - id = sendApplicationActionResult.data.id - } + const id = sendApplicationActionResult?.data?.id ?? 'unknown'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.type ApplicationResult = { data: { id: string } } const sendApplicationActionResult = application.externalData[ApiActions.createApplication] as ApplicationResult | undefined const id = sendApplicationActionResult?.data?.id ?? 'unknown'
78-81:
⚠️ Potential issueRemove @ts-ignore and add proper type safety
Using @ts-ignore masks potential type issues. The code should be properly typed.
Add proper type handling:
- // @ts-ignore - id = sendApplicationActionResult.data.id + interface ApplicationResult { + data: { id: string } + } + id = (sendApplicationActionResult as ApplicationResult)?.data?.id ?? 'unknown'Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/displayFieldSubsection.ts (1)
52-57:
⚠️ Potential issueAdd input validation for currency calculations.
The direct conversion of string values to numbers without validation could lead to NaN results if inputs are empty or invalid.
value: (answers) => { - const value1 = Number(getValueViaPath<string>(answers, 'input1')) - const value2 = Number(getValueViaPath<string>(answers, 'input2')) - const value3 = Number(getValueViaPath<string>(answers, 'input3')) - return `${value1 + value2 + value3}` + const value1 = getValueViaPath<string>(answers, 'input1') + const value2 = getValueViaPath<string>(answers, 'input2') + const value3 = getValueViaPath<string>(answers, 'input3') + if (!value1 || !value2 || !value3) return '' + const sum = [value1, value2, value3] + .map(v => Number(v)) + .reduce((acc, curr) => acc + curr, 0) + return `${sum}`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.value: (answers) => { const value1 = getValueViaPath<string>(answers, 'input1') const value2 = getValueViaPath<string>(answers, 'input2') const value3 = getValueViaPath<string>(answers, 'input3') if (!value1 || !value2 || !value3) return '' const sum = [value1, value2, value3] .map(v => Number(v)) .reduce((acc, curr) => acc + curr, 0) return `${sum}` },libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/textInputSubsection.ts (2)
104-108: 🛠️ Refactor suggestion
Replace tel variant with buildPhoneField as suggested in the placeholder.
For consistency and better validation, use the dedicated buildPhoneField component instead of the tel variant.
-buildTextField({ - id: 'telTextInput', - title: '', - placeholder: 'Variant tel (try to use buildPhoneField)', - variant: 'tel', -}), +buildPhoneField({ + id: 'phoneInput', + title: '', + placeholder: 'Phone number', +}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.buildPhoneField({ id: 'phoneInput', title: '', placeholder: 'Phone number', }),
15-46: 🛠️ Refactor suggestion
Add aria-labels for better accessibility.
Text inputs should include aria-labels, especially when the title is empty, to improve accessibility for screen readers.
buildTextField({ id: 'textInput', title: '', + ariaLabel: 'Basic text input', placeholder: 'The most basic text input', }),Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/sliderSubsection.ts (1)
21-25: 🛠️ Refactor suggestion
⚠️ Potential issueFix color format and extract repeated style calculation.
The background color is missing the # prefix, and the calculateCellStyle function is duplicated across all slider implementations.
+const defaultCellStyle = { + background: '#ccccd8', +} + +const calculateDefaultCellStyle = () => defaultCellStyle + calculateCellStyle: () => { return { - background: 'ccccd8', + ...defaultCellStyle, } },Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/assets/plate-110-510.tsx (1)
3-40: 🛠️ Refactor suggestion
Consider creating a shared base component for plates.
The plate components share similar structure and properties, suggesting an opportunity for a shared abstraction.
Consider creating a base component:
// BasePlate.tsx interface BasePlateProps { width: number; height: number; fillColor?: string; strokeColor?: string; children: React.ReactNode; } export const BasePlate: FC<BasePlateProps> = ({ width, height, fillColor = '#E6CF00', strokeColor = '#00003C', children }) => ( <svg width={width} height={height} viewBox={`0 0 ${width} ${height}`} fill="none" xmlns="http://www.w3.org/2000/svg" > <rect x="0.5" y="0.5" width={width - 1} height={height - 1} rx="6.72201" fill={fillColor} stroke={strokeColor} /> {children} </svg> );Then use it in plate components:
export const plate110: FC = () => ( <BasePlate width={220} height={63}> <path d="..." fill="#00003C" /> {/* other paths */} </BasePlate> );libs/application/templates/reference-template/src/assets/plate-200-280.tsx (2)
3-3: 🛠️ Refactor suggestion
Remove unused children props
The component accepts
React.PropsWithChildren<unknown>but doesn't utilize the children props in its implementation.-export const plate200: FC<React.PropsWithChildren<unknown>> = () => ( +export const plate200: FC = () => (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export const plate200: FC = () => (
4-10: 🛠️ Refactor suggestion
Add accessibility attributes to SVG
The SVG lacks accessibility attributes which are essential for screen readers.
<svg width="158" height="119" viewBox="0 0 158 119" fill="none" xmlns="http://www.w3.org/2000/svg" + role="img" + aria-label="Plate 200" >📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<svg width="158" height="119" viewBox="0 0 158 119" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="Plate 200" >libs/application/templates/funding-government-projects/src/forms/FundingGovernmentProjectsForm.ts (1)
126-126: 🛠️ Refactor suggestion
Avoid empty title strings
Empty strings for titles may impact accessibility and user experience. Consider removing the property if no title is needed, or provide a meaningful title.
-title: '',Committable suggestion skipped: line range outside the PR's diff.
libs/application/templates/reference-template/src/forms/exampleForm/index.ts
Show resolved
Hide resolved
...templates/reference-template/src/forms/exampleForm/simpleInputsSection/checkboxSubsection.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
libs/application/templates/reference-template/src/utils/README.md (1)
5-5: Enhance documentation with specific guidelines and examples.While the organization guidance is good, consider enhancing the documentation to better align with our coding guidelines:
-The organization of this folder is up to the developer. Try to aim for a happy medium between small focused files and too many files that bloat the folder. The aim is that a developer can enter this folder and understand what the utils are for and have a nice overview of the utils. +The organization of this folder is up to the developer. Try to aim for a happy medium between small, focused files and too many files that bloat the folder. The aim is that a developer can enter this folder and understand what the utils are for and have a nice overview of the utils. + +## Guidelines + +- Export TypeScript types and interfaces for public utilities +- Ensure utilities are tree-shakeable for optimal bundling +- Follow naming conventions: + - Utilities: camelCase (e.g., `formatDate`) + - Types/Interfaces: PascalCase (e.g., `DateFormat`) + - Constants: UPPER_SNAKE_CASE (e.g., `DEFAULT_DATE_FORMAT`) + +## Examples + +```typescript +// dateUtils.ts +export interface DateFormat { + format: string; + locale: string; +} + +export const formatDate = (date: Date, options: DateFormat): string => { + // Implementation +}; +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: Possible missing comma found.
Context: .... Try to aim for a happy medium between small focused files and too many files that b...(AI_HYDRA_LEO_MISSING_COMMA)
libs/application/templates/reference-template/README.md (2)
69-80: Enhance clarity and professionalism of coding guidelinesWhile the guidelines provide valuable direction, consider these improvements:
- Rephrase the introduction more professionally: "The aim is to ensure consistent coding patterns across all applications, enabling developers familiar with the application system to navigate any application with ease."
- Expand the
getValueViaPathexample to show its benefits:// Instead of deeply nested access const value = answers?.some?.deeply?.nested?.value ?? fallback; // Use type-safe accessor const value = getValueViaPath<string>( answers, 'some.deeply.nested.value', fallback );
- Clarify the stepper guideline by explaining the rationale: "Maintain accurate step visualization: The stepper should only display steps within the current form to avoid confusion. For example, the main form's first step should not show a back button to prerequisites, as they are part of a different form flow."
Would you like me to propose a complete revision of this section?
🧰 Tools
🪛 LanguageTool
[style] ~72-~72: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...is to have all applications to be coded in a similar way, so that every developer that is famili...(IN_A_X_MANNER)
[style] ~72-~72: Consider using “who” when you are referring to a person instead of an object.
Context: ... a similar way, so that every developer that is familiar with the application system...(THAT_WHO)
[uncategorized] ~76-~76: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...able. Note that this function is generic and a type can be provided to make sure the...(COMMA_COMPOUND_SENTENCE_2)
81-93: Enhance folder structure documentation with examplesThe folder structure provides a good foundation but could benefit from more detailed explanations and examples:
## Folder structure |-- assets/ # Static assets (images, icons, etc.) | |-- images/ # e.g., form-header.svg | |-- icons/ # e.g., custom-icon.svg | |-- components/ # Reusable React components | |-- FormSection/ # e.g., index.tsx, styles.ts | |-- CustomInput/ # e.g., index.tsx, types.ts | |-- dataProviders/ # Data fetching and state management | |-- userProvider.ts # e.g., getUserData, updateUserData | |-- fields/ # Custom form field components | |-- index.ts # Exports all fields | |-- CustomField/ # e.g., index.tsx, validation.ts | ... (continue for other folders)Consider adding:
- Naming conventions for files and folders
- Guidelines for index files
- Examples of typical files in each folder
- Dependencies between folders
Would you like me to propose a complete revision of the folder structure documentation?
libs/application/templates/reference-template/src/forms/README.md (3)
1-7: Fix typo and improve clarity in the introductionThere are a few issues in the introduction:
- "complecated" should be "complicated"
- Missing comma before "and" in the compound sentence
Apply this diff:
# The forms folder This folder contains all the forms that are used by the application. At minimum there should be a prerequisites form, application form and a confirmation form to reflect the possible states of an application. -For more complecated applications you could have different forms depending on the user type and you could have more states like approved, rejected, waiting to assign... +For more complicated applications, you could have different forms depending on the user type, and you could have more states like approved, rejected, waiting to assign...🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...fferent forms depending on the user type and you could have more states like approve...(COMMA_COMPOUND_SENTENCE)
10-13: Enhance organization guidelines with examplesWhile the naming conventions are clear, it would be helpful to include examples of correct and incorrect naming to reinforce the guidelines.
Consider adding examples like:
All forms should be in a folder with the same name as the form. The folder and files in it should follow camelCase, PascalCase should be reserved for React components. A simple form with one screen, like the prerequisites form can be just a single file. -Form with more than one section and possibly subsections should be broken down into one file per screen. +A form with more than one section and possibly subsections should be broken down into one file per screen. + +Examples: +✅ Good: + - Folder: personalInfoForm + - File: personalInfoForm.tsx + - Component: PersonalInfoForm + +❌ Bad: + - Folder: PersonalInfoForm + - File: PersonalInfo.tsx + - Component: personalInfoForm🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: A comma might be missing here.
Context: ...with one screen, like the prerequisites form can be just a single file. Form with mo...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~11-~11: Possible missing article found.
Context: ...uisites form can be just a single file. Form with more than one section and possibly...(AI_HYDRA_LEO_MISSING_A)
1-32: Consider adding TypeScript examples for form buildersTo align with the coding guidelines for libs/** regarding TypeScript usage, consider adding TypeScript interface examples for the form builders.
Add a new section showing TypeScript interfaces:
// Example TypeScript interfaces for form builders interface FormConfig { id: string; title: string; sections: SectionConfig[]; } interface SectionConfig { id: string; title: string; subsections: SubSectionConfig[]; } interface SubSectionConfig { id: string; title: string; fields: FormField[]; } // Example usage: const buildForm = (): FormConfig => { return { id: 'applicationForm', title: 'Application Form', sections: [/* ... */], } }🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...fferent forms depending on the user type and you could have more states like approve...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~11-~11: A comma might be missing here.
Context: ...with one screen, like the prerequisites form can be just a single file. Form with mo...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~11-~11: Possible missing article found.
Context: ...uisites form can be just a single file. Form with more than one section and possibly...(AI_HYDRA_LEO_MISSING_A)
libs/application/templates/reference-template/src/fields/README.md (4)
1-4: Consider enhancing the introduction with more context.While the current introduction is clear, it would be helpful to add more context about how these custom components fit into the broader reference template system and their role in application development.
# The fields folder This folder contains all custom components that are used by the application. + +These components serve as building blocks for creating standardized forms and interfaces within the reference template system. They are designed to maintain consistency across different applications while providing flexibility for custom implementations when needed.
5-10: Fix grammatical issues and enhance organization guidelines.The organization guidelines are well-structured but contain some grammatical issues.
## Organisation -All components should be in a folder that holds all files for that component. This includes the .tsx file, possibly a .css.ts file and maybe others. - The folders should be named like the component, but with the first letter in lowercase (camelCase), and then the .tsx and .css.ts files should be capitalized (PascalCase). -The folder should have an index.ts file that re-exports all the components. -The index.ts file in the /src folder should then re-export the components in the /fields folder for the template loader. +Each component should have its own folder containing all related files (.tsx, .css.ts, etc.). + +Naming conventions: +- Component folders: camelCase (e.g., `customField`) +- Component files: PascalCase (e.g., `CustomField.tsx`, `CustomField.css.ts`) + +Export structure: +- Each component folder must include an `index.ts` that re-exports its components +- The `/src/index.ts` must re-export all components from the `/fields` folder for the template loader + +Example: +``` +/fields + /customField + CustomField.tsx + CustomField.css.ts + index.ts + index.ts +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... The folders should be named like the component, but with the first letter in lowercase...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
12-21: Enhance usage guidelines with examples and fix grammatical issues.While the guidelines effectively promote component reusability, they could benefit from better formatting, grammar fixes, and concrete examples.
-## Useage of custom components +## Usage of Custom Components Before creating a custom component, you should: -1. Try to use the shared components, `buildTextField`, `buildCheckboxField`, `buildSelectField`, `buildFileUploadField` and so on. This is most preferable to make the look and feel of the application more consistent and uniform. -2. If the shared components almost fullfill your needs but you need something more, consider consulting with the designer of the application and try to adjust the design to the built in components. -3. If the design can not be adjusted to the built in components, then consult Norda if a shared component can possibly be adjusted or expanded to fulfill your needs. -4. Is there another application that has made a similar custom component before? If so, then it should be a shared component. -5. If you still need a new component, ask yourself if this is a component that another application might need in the future. If so make the new component shared. -6. Make a custom component if none of the above apply. +1. Use existing shared components whenever possible: + ```typescript + // Example: Using shared components + buildTextField('fullName', { + label: 'Full Name', + required: true + }) + ``` + Common shared components: + - `buildTextField` + - `buildCheckboxField` + - `buildSelectField` + - `buildFileUploadField` + +2. If shared components almost fulfill your needs: + - Consult the application designer + - Try to adjust the design to use built-in components + +3. For minor adjustments to shared components: + - Consult Norda about expanding existing components + - Document the new requirements + +4. Check for similar components: + - Review other applications in the ecosystem + - Consider converting existing custom components to shared ones + +5. For new components: + - Evaluate potential reuse across applications + - If reusable, create as a shared component + +6. Create a custom component only if: + - None of the above options apply + - The component is truly application-specific🧰 Tools
🪛 LanguageTool
[style] ~17-~17: This phrase is redundant. Consider writing “consulting”.
Context: ...s but you need something more, consider consulting with the designer of the application and try...(CONSULT_WITH)
[grammar] ~17-~17: A hyphen is missing in the adjective “built-in”.
Context: ...ion and try to adjust the design to the built in components. 3. If the design can not be...(BUILT_IN_HYPHEN)
[style] ~18-~18: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e built in components. 3. If the design can not be adjusted to the built in components,...(CAN_NOT_PREMIUM)
[grammar] ~18-~18: A hyphen is missing in the adjective “built-in”.
Context: ...f the design can not be adjusted to the built in components, then consult Norda if a sha...(BUILT_IN_HYPHEN)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...lication might also need in the future. If so make the new component shared. 6. Ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ation might also need in the future. If so make the new component shared. 6. Make ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
1-21: Add explicit guidance for TypeScript types and tree-shaking.While the documentation covers component organization well, it should explicitly address TypeScript type exports and tree-shaking considerations from the coding guidelines.
+## TypeScript and Bundling Guidelines + +### Type Definitions +- Export component types and interfaces from your `index.ts` files +- Use explicit type annotations for component props + +Example: +```typescript +// CustomField.tsx +export interface CustomFieldProps { + label: string; + required?: boolean; +} + +// index.ts +export type { CustomFieldProps } from './CustomField'; +``` + +### Tree-Shaking Optimization +- Export components individually, not as a default export +- Avoid barrel exports that could prevent tree-shaking +- Keep components focused and avoid unnecessary dependencies + +Example: +```typescript +// Good - enables tree-shaking +export { CustomField } from './CustomField'; + +// Avoid - may prevent tree-shaking +export * from './CustomField'; +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~8-~8: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... The folders should be named like the component, but with the first letter in lowercase...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~17-~17: This phrase is redundant. Consider writing “consulting”.
Context: ...s but you need something more, consider consulting with the designer of the application and try...(CONSULT_WITH)
[grammar] ~17-~17: A hyphen is missing in the adjective “built-in”.
Context: ...ion and try to adjust the design to the built in components. 3. If the design can not be...(BUILT_IN_HYPHEN)
[style] ~18-~18: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e built in components. 3. If the design can not be adjusted to the built in components,...(CAN_NOT_PREMIUM)
[grammar] ~18-~18: A hyphen is missing in the adjective “built-in”.
Context: ...f the design can not be adjusted to the built in components, then consult Norda if a sha...(BUILT_IN_HYPHEN)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...lication might also need in the future. If so make the new component shared. 6. Ma...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ation might also need in the future. If so make the new component shared. 6. Make ...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
libs/application/templates/reference-template/README.md(1 hunks)libs/application/templates/reference-template/src/assets/README.md(1 hunks)libs/application/templates/reference-template/src/components/Logo/Logo.tsx(1 hunks)libs/application/templates/reference-template/src/dataProviders/README.md(1 hunks)libs/application/templates/reference-template/src/dataProviders/index.ts(1 hunks)libs/application/templates/reference-template/src/fields/README.md(1 hunks)libs/application/templates/reference-template/src/fields/index.ts(1 hunks)libs/application/templates/reference-template/src/forms/README.md(1 hunks)libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/checkboxSubsection.ts(1 hunks)libs/application/templates/reference-template/src/forms/prerequisitesForm/prerequisitesForm.ts(1 hunks)libs/application/templates/reference-template/src/graphql/README.md(1 hunks)libs/application/templates/reference-template/src/lib/README.md(1 hunks)libs/application/templates/reference-template/src/utils/README.md(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- libs/application/templates/reference-template/src/assets/README.md
- libs/application/templates/reference-template/src/dataProviders/README.md
- libs/application/templates/reference-template/src/graphql/README.md
- libs/application/templates/reference-template/src/dataProviders/index.ts
- libs/application/templates/reference-template/src/lib/README.md
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/application/templates/reference-template/src/fields/index.ts
- libs/application/templates/reference-template/src/forms/exampleForm/simpleInputsSection/checkboxSubsection.ts
- libs/application/templates/reference-template/src/forms/prerequisitesForm/prerequisitesForm.ts
- libs/application/templates/reference-template/src/components/Logo/Logo.tsx
🧰 Additional context used
📓 Path-based instructions (4)
libs/application/templates/reference-template/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/fields/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/forms/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/utils/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/application/templates/reference-template/README.md
[style] ~72-~72: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...is to have all applications to be coded in a similar way, so that every developer that is famili...
(IN_A_X_MANNER)
[style] ~72-~72: Consider using “who” when you are referring to a person instead of an object.
Context: ... a similar way, so that every developer that is familiar with the application system...
(THAT_WHO)
[uncategorized] ~76-~76: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...able. Note that this function is generic and a type can be provided to make sure the...
(COMMA_COMPOUND_SENTENCE_2)
libs/application/templates/reference-template/src/fields/README.md
[uncategorized] ~8-~8: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ... The folders should be named like the component, but with the first letter in lowercase...
(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[style] ~17-~17: This phrase is redundant. Consider writing “consulting”.
Context: ...s but you need something more, consider consulting with the designer of the application and try...
(CONSULT_WITH)
[grammar] ~17-~17: A hyphen is missing in the adjective “built-in”.
Context: ...ion and try to adjust the design to the built in components. 3. If the design can not be...
(BUILT_IN_HYPHEN)
[style] ~18-~18: Unless you want to emphasize “not”, use “cannot” which is more common.
Context: ...e built in components. 3. If the design can not be adjusted to the built in components,...
(CAN_NOT_PREMIUM)
[grammar] ~18-~18: A hyphen is missing in the adjective “built-in”.
Context: ...f the design can not be adjusted to the built in components, then consult Norda if a sha...
(BUILT_IN_HYPHEN)
[style] ~20-~20: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...lication might also need in the future. If so make the new component shared. 6. Ma...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[uncategorized] ~20-~20: A comma might be missing here.
Context: ...ation might also need in the future. If so make the new component shared. 6. Make ...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
libs/application/templates/reference-template/src/forms/README.md
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...fferent forms depending on the user type and you could have more states like approve...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~11-~11: A comma might be missing here.
Context: ...with one screen, like the prerequisites form can be just a single file. Form with mo...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~11-~11: Possible missing article found.
Context: ...uisites form can be just a single file. Form with more than one section and possibly...
(AI_HYDRA_LEO_MISSING_A)
libs/application/templates/reference-template/src/utils/README.md
[uncategorized] ~5-~5: Possible missing comma found.
Context: .... Try to aim for a happy medium between small focused files and too many files that b...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
libs/application/templates/reference-template/src/utils/README.md (1)
1-3: LGTM! Clear and concise introduction.
The title and purpose are well-defined, providing a clear overview of the folder's contents.
...application/templates/funding-government-projects/src/forms/FundingGovernmentProjectsForm.ts
Show resolved
Hide resolved
libs/application/templates/reference-template/src/forms/README.md
Outdated
Show resolved
Hide resolved
...plates/reference-template/src/forms/exampleForm/commonActionsSection/conditionsSubsection.ts
Show resolved
Hide resolved
libs/application/templates/reference-template/src/forms/exampleForm/index.ts
Show resolved
Hide resolved
...eference-template/src/forms/exampleForm/tablesAndRepeatersSection/tableRepeaterSubsection.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/SliderFormField/SliderFormField.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Þórarinn Gunnar Árnason <torarinng@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (12)
libs/application/templates/reference-template/src/assets/plate-110-510.tsx (2)
1-1: Add TypeScript type definitions for better maintainabilityThe component should be explicitly typed as a React FC (FunctionComponent) and consider accepting props for flexibility.
-export const plate110 = () => ( +import { FC } from 'react' + +interface Plate110Props { + className?: string + width?: number + height?: number +} + +export const plate110: FC<Plate110Props> = ({ + className, + width = 220, + height = 63 +}) => (
2-10: Consider making SVG dimensions configurableThe SVG has hardcoded dimensions. For better reusability across different contexts, consider making these configurable through props while maintaining the aspect ratio.
<svg - width="220" - height="63" + width={width} + height={height} viewBox="0 0 220 63" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="Plate 110 510" + className={className} >libs/application/templates/reference-template/README.md (3)
72-72: Improve readability of the introduction sentenceThe sentence structure can be improved for better clarity.
-The aim is to have all applications to be coded in a similar way, so that every developer that is familiar with the application system can open any application and everything is consistent and feels familiar. +The aim is to have all applications coded similarly, so that every developer who is familiar with the application system can open any application and find it consistent and familiar.🧰 Tools
🪛 LanguageTool
[style] ~72-~72: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...is to have all applications to be coded in a similar way, so that every developer that is famili...(IN_A_X_MANNER)
[style] ~72-~72: Consider using “who” when you are referring to a person instead of an object.
Context: ... a similar way, so that every developer that is familiar with the application system...(THAT_WHO)
74-76: Enhance clarity of TypeScript guidelinesThe TypeScript-related guidelines could be more specific and include examples.
-Reduce the use of the "as" and "any" keywords as much as possible. Both of those keywords are tricking the linter to accept code that would otherwise throw errors and might cause hard to trace bugs. +Avoid using "as" and "any" keywords as they bypass TypeScript's type checking. Instead: +- Use proper type definitions +- Leverage type inference +- Use type guards when type narrowing is needed +Example: +```typescript +// Instead of +const value = someValue as string; + +// Use +if (typeof someValue === 'string') { + const value = someValue; // Type is inferred as string +} +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~76-~76: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...able. Note that this function is generic and a type can be provided to make sure the...(COMMA_COMPOUND_SENTENCE_2)
80-80: Clarify stepper navigation guidelinesThe guidance about fake steps and back buttons could be more explicit.
-Don't use fake steps, stepper should only be showing steps of the current form. On the first step in the main form, there shouldn't be a back button or the illusion that you can go back to the prerequsites step. +Guidelines for stepper navigation: +1. Only show actual steps within the current form +2. Remove the back button on the first step of the main form +3. Maintain clear separation between prerequisites and main form steps +4. Ensure each step represents a meaningful user actionlibs/application/templates/reference-template/src/forms/README.md (3)
1-5: Add TypeScript interfaces for form typesTo align with the coding guidelines for libs/** patterns, consider adding TypeScript interface documentation for the different form types. This will enhance reusability across NextJS apps.
Add the following type definitions:
/** * Base interface for all forms */ interface BaseForm { id: string; title: string; // Add common properties } /** * Prerequisites form type */ interface PrerequisitesForm extends BaseForm { type: 'prerequisites'; // Add specific properties } // Similar interfaces for ApplicationForm and ConfirmationForm
10-12: Improve clarity and fix grammar in organization sectionThe section needs minor improvements for better readability and grammar.
-A simple form with one screen, like the prerequisites form can be just a single file. -Form with more than one section and possibly subsections should be broken down into one file per screen. +A simple form with one screen, like the prerequisites form, can be just a single file. +A form with more than one section and possibly subsections should be broken down into one file per screen.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Possible missing article found.
Context: ...uisites form can be just a single file. Form with more than one section and possibly...(AI_HYDRA_LEO_MISSING_A)
46-48: Improve phrasing and fix style issuesSeveral style improvements are needed in the building blocks explanation.
-BuildMultiField is a wrapper around many buildField functions. if a buildSection or buildSubSection doesn't have a buildMultiField, it will only display one field at a time and stepping through the screens will be strange in regards to the stepper. -In most cases you want to use one buildMultiField as the child of a buildSection or buildSubSection since in most cases you want to display multiple fields at a time on the screen. +BuildMultiField is a wrapper around many buildField functions. If a buildSection or buildSubSection doesn't have a buildMultiField, it will only display one field at a time, and navigation through the screens will be inconsistent with the stepper. +In most cases, it's recommended to use one buildMultiField as the child of a buildSection or buildSubSection to display multiple fields simultaneously on the screen.🧰 Tools
🪛 LanguageTool
[misspelling] ~46-~46: Although “in regards to” is sometimes used in casual speech, it is typically considered a nonstandard phrase.
Context: ...ing through the screens will be strange in regards to the stepper. In most cases you want to...(IN_OR_WITH_REGARDS_TO_OF)
[formatting] ~47-~47: Insert a comma after ‘cases’: “In most cases,”?
Context: ... be strange in regards to the stepper. In most cases you want to use one buildMultiField as ...(IN_MOST_CASES_COMMA)
[style] ~48-~48: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...buildSubSection since in most cases you want to display multiple fields at a time on th...(REP_WANT_TO_VB)
libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.css.ts (1)
8-10: Consider making bottomBorderRadius more reusableWhile the current implementation works, consider making this style more flexible by accepting radius values as parameters or creating a utility function. This would enhance reusability across different components.
Example approach:
export const createBorderRadius = (corners: 'top' | 'bottom' | 'all') => style({ borderRadius: corners === 'bottom' ? `0 0 ${theme.border.radius.large} ${theme.border.radius.large}` : corners === 'top' ? `${theme.border.radius.large} ${theme.border.radius.large} 0 0` : theme.border.radius.large })libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx (1)
15-21: Consider memoization for performance optimizationIf this component is used in a list or renders frequently, consider using React.memo to prevent unnecessary re-renders.
- export const ExampleCustomComponent = ({ + export const ExampleCustomComponent = React.memo(({ application, field, }: Props & FieldBaseProps) => { const { formatMessage } = useLocale() const { someData } = field.props if (!someData) return null // ... rest of the component - } + })libs/application/templates/reference-template/src/fields/Overview/Overview.tsx (2)
51-61: Use optional chaining for better code quality.The conditional rendering can be simplified using optional chaining.
-{tableRepeaterAnswers && - tableRepeaterAnswers.map((ans, i) => { +{tableRepeaterAnswers?.map((ans, i) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 51-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
64-64: Address the TODO comment.The comment indicates incomplete implementation. Please clarify if more review groups are needed.
Would you like me to help implement additional review groups or create a GitHub issue to track this task?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
libs/application/templates/reference-template/README.md(1 hunks)libs/application/templates/reference-template/src/assets/plate-110-510.tsx(1 hunks)libs/application/templates/reference-template/src/assets/plate-155-305.tsx(1 hunks)libs/application/templates/reference-template/src/assets/plate-200-280.tsx(1 hunks)libs/application/templates/reference-template/src/fields/Overview/Overview.tsx(1 hunks)libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.css.ts(1 hunks)libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx(1 hunks)libs/application/templates/reference-template/src/fields/index.ts(1 hunks)libs/application/templates/reference-template/src/forms/README.md(1 hunks)libs/application/templates/reference-template/src/forms/exampleForm/customSection/customSection.ts(1 hunks)libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/descriptionSubsection.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- libs/application/templates/reference-template/src/assets/plate-200-280.tsx
- libs/application/templates/reference-template/src/assets/plate-155-305.tsx
- libs/application/templates/reference-template/src/forms/exampleForm/noInputFieldsSection/descriptionSubsection.ts
- libs/application/templates/reference-template/src/forms/exampleForm/customSection/customSection.ts
🧰 Additional context used
📓 Path-based instructions (7)
libs/application/templates/reference-template/src/fields/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/assets/plate-110-510.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.css.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/forms/README.md (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 LanguageTool
libs/application/templates/reference-template/README.md
[style] ~72-~72: Consider replacing this phrase with the adverb “similarly” to avoid wordiness.
Context: ...is to have all applications to be coded in a similar way, so that every developer that is famili...
(IN_A_X_MANNER)
[style] ~72-~72: Consider using “who” when you are referring to a person instead of an object.
Context: ... a similar way, so that every developer that is familiar with the application system...
(THAT_WHO)
[uncategorized] ~76-~76: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...able. Note that this function is generic and a type can be provided to make sure the...
(COMMA_COMPOUND_SENTENCE_2)
libs/application/templates/reference-template/src/forms/README.md
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...fferent forms depending on the user type and you could have more states like approve...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~11-~11: Possible missing article found.
Context: ...uisites form can be just a single file. Form with more than one section and possibly...
(AI_HYDRA_LEO_MISSING_A)
[uncategorized] ~38-~38: Possible missing comma found.
Context: .... The root of each form is a buildForm function and that is the only thing that can be ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~42-~42: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ion will be displayed in the stepper as top level section. The children array of a buildS...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[misspelling] ~46-~46: Although “in regards to” is sometimes used in casual speech, it is typically considered a nonstandard phrase.
Context: ...ing through the screens will be strange in regards to the stepper. In most cases you want to...
(IN_OR_WITH_REGARDS_TO_OF)
[formatting] ~47-~47: Insert a comma after ‘cases’: “In most cases,”?
Context: ... be strange in regards to the stepper. In most cases you want to use one buildMultiField as ...
(IN_MOST_CASES_COMMA)
[style] ~48-~48: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...buildSubSection since in most cases you want to display multiple fields at a time on th...
(REP_WANT_TO_VB)
🪛 Biome (1.9.4)
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx
[error] 51-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
libs/application/templates/reference-template/src/forms/README.md (2)
6-6: Fix grammar in complex applications description
This matches the previous suggestion about different forms for user types. Add a comma before "and" to improve readability.
-For more complicated applications you could have different forms depending on the user type and you could have more states like approved, rejected, waiting to assign...
+For more complicated applications, you could have different forms depending on the user type, and you could have more states like approved, rejected, waiting to assign...🧰 Tools
🪛 LanguageTool
[uncategorized] ~6-~6: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...fferent forms depending on the user type and you could have more states like approve...
(COMMA_COMPOUND_SENTENCE)
19-30: Document the purpose of build functions in the example structure
Add comments explaining the purpose of each build function in the example structure.
| /applicationForm
-| |-- index.ts (This file has a buildForm function)
+| |-- index.ts (This file has a buildForm function) // Assembles the complete form configuration
| |-- /section1
-| | |-- index.ts (This file has a buildSection function and imports the subsection childs)
+| | |-- index.ts (This file has a buildSection function and imports the subsection children) // Manages section-level logic
-| | |-- subsection1.ts (This file has a buildSubSection function)
+| | |-- subsection1.ts (This file has a buildSubSection function) // Handles individual form field groupslibs/application/templates/reference-template/src/fields/index.ts (1)
3-4: LGTM! Exports are well-organized
The new exports follow the established pattern and maintain consistency with the existing exports.
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx (1)
1-8: LGTM! Imports are well-organized and tree-shakeable.
The imports follow best practices by using named imports, which enables effective tree-shaking during bundling.
...on/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx
Outdated
Show resolved
Hide resolved
...on/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx
Show resolved
Hide resolved
...on/templates/reference-template/src/fields/exampleCustomComponent/ExampleCustomComponent.tsx
Show resolved
Hide resolved
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx
Show resolved
Hide resolved
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx
Show resolved
Hide resolved
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx
Show resolved
Hide resolved
libs/application/templates/reference-template/src/fields/Overview/Overview.tsx
Show resolved
Hide resolved
…and.is into feat/update-example-form-2
What
Update the reference template so that it showcases what can be done with the application system and how teams should write applications
Why
Reference template was outdated
Checklist:
Summary by CodeRabbit
New Features
buildAlertMessageFieldand enhancedbuildSliderField.Documentation
Bug Fixes
Chores
Style