-
Notifications
You must be signed in to change notification settings - Fork 14
Rework cozy-ui-plus Contacts add modal #2980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
659f7c0
13a5006
65ad3a1
c6cba22
f04795d
ad7d94b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| # Configuration for AI agents | ||
|
|
||
| ## ESLint Configuration | ||
|
|
||
| This project uses ESLint rules from `cozy-libs`. When generating code, strictly follow these rules: | ||
|
|
||
| ### Configuration files | ||
|
|
||
| - `config/eslint-config-cozy-app/basics.js` | ||
| - `config/eslint-config-cozy-app/react.js` | ||
|
|
||
| ### Important rules to follow | ||
|
|
||
| #### Prettier (formatting) | ||
|
|
||
| ```javascript | ||
| 'prettier/prettier': [ | ||
| 'error', | ||
| { | ||
| arrowParens: 'avoid', // No parentheses for single-parameter arrow functions | ||
| endOfLine: 'auto', // Automatic line ending | ||
| semi: false, // NO SEMICOLONS at end of lines | ||
| singleQuote: true, // Single quotes | ||
| trailingComma: 'none' // NO TRAILING COMMAS | ||
| } | ||
| ] | ||
| ``` | ||
|
|
||
| #### Import/Order (import order) | ||
|
|
||
| - Mandatory alphabetical order (`alphabetize: { order: 'asc' }`) | ||
| - Import groups in this order: | ||
| 1. `builtin` (Node.js modules) | ||
| 2. `external` (npm packages) | ||
| 3. `internal` (project internal imports) | ||
| 4. `['parent', 'sibling', 'index']` (relative imports) | ||
| - Special pattern for cozy-\_ and twake-\_ (position after external) | ||
| - Empty line between each group (`newlines-between: 'always'`) | ||
|
|
||
| #### React | ||
|
|
||
| - `react/prop-types`: off (disabled) | ||
| - `react/jsx-curly-brace-presence`: no unnecessary braces for props and children | ||
| - No `display-name` in .spec files | ||
|
|
||
| #### TypeScript (if applicable) | ||
|
|
||
| - `explicit-function-return-type`: required | ||
| - `no-explicit-any`: forbidden | ||
|
|
||
| ### Compliant code example | ||
|
|
||
| ```javascript | ||
| import React from "react"; | ||
|
|
||
| import Button from "cozy-ui/transpiled/react/Buttons"; | ||
| import cx from "classnames"; | ||
|
|
||
| import FieldInput from "./FieldInput"; | ||
| import { makeIsRequiredError } from "./helpers"; | ||
|
|
||
| const MyComponent = ({ name, label }) => { | ||
| const isError = makeIsRequiredError(name); | ||
|
|
||
| return ( | ||
| <div className={cx("u-mt-1", { "u-flex": isError })}> | ||
| <FieldInput name={name} label={label} /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export default MyComponent; | ||
| ``` | ||
|
|
||
| Note: | ||
|
|
||
| - No semicolons | ||
| - Single quotes | ||
| - No trailing comma | ||
| - Alphabetical import order (React before Button, Button before cx) | ||
| - Empty line between import groups (builtin/external, external/internal, etc.) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,16 +4,32 @@ import PropTypes from 'prop-types' | |
| import React, { useState } from 'react' | ||
| import { Field } from 'react-final-form' | ||
|
|
||
| import { useBreakpoints } from 'cozy-ui/transpiled/react/providers/Breakpoints' | ||
| import { useI18n, useExtendI18n } from 'twake-i18n' | ||
|
|
||
| import FieldInputWrapper from './FieldInputWrapper' | ||
| import HasValueCondition from './HasValueCondition' | ||
| import { RelatedContactList } from './RelatedContactList' | ||
| import RemoveButton from './RemoveButton' | ||
| import { locales } from './locales' | ||
| import styles from './styles.styl' | ||
| import ContactAddressDialog from '../ContactAddressDialog' | ||
| import { fieldInputAttributesTypes, labelPropTypes } from '../types' | ||
|
|
||
| const FieldAndRemove = ({ showRemove, onRemove, ...props }) => { | ||
| const { isMobile } = useBreakpoints() | ||
|
|
||
| if (isMobile) { | ||
| return ( | ||
| <div className="u-flex u-flex-items-center u-w-100"> | ||
| <Field {...props} /> | ||
| {showRemove && <RemoveButton onRemove={onRemove} />} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| return <Field {...props} /> | ||
| } | ||
|
|
||
| const FieldInput = ({ | ||
| name, | ||
| labelProps, | ||
|
|
@@ -22,6 +38,8 @@ const FieldInput = ({ | |
| contacts, | ||
| contact, | ||
| error, | ||
| onRemove, | ||
| showRemove, | ||
| helperText, | ||
| label, | ||
| isInvisible | ||
|
|
@@ -33,6 +51,7 @@ const FieldInput = ({ | |
| useState(false) | ||
| useExtendI18n(locales) | ||
| const { t } = useI18n() | ||
| const { isMobile } = useBreakpoints() | ||
|
|
||
| const handleClick = () => { | ||
| if (name.includes('address')) { | ||
|
|
@@ -50,14 +69,12 @@ const FieldInput = ({ | |
|
|
||
| return ( | ||
| <div | ||
| className={cx( | ||
| className, | ||
| styles['contact-form-field__wrapper'], | ||
| 'u-flex-column-s', | ||
| { 'u-flex': !isInvisible, 'u-dn': isInvisible } | ||
| )} | ||
| className={cx(className, 'u-flex-column-s u-flex-items-center u-w-100', { | ||
| 'u-flex': !isInvisible, | ||
| 'u-dn': isInvisible | ||
| })} | ||
| > | ||
| <Field | ||
| <FieldAndRemove | ||
| error={error} | ||
| helperText={helperText} | ||
| label={label} | ||
|
|
@@ -66,6 +83,8 @@ const FieldInput = ({ | |
| name={name} | ||
| contact={contact} | ||
| component={FieldInputWrapper} | ||
| showRemove={showRemove} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you should do It is managed differently between desktop and mobile.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's a good point 🤔 I prefer to separate responsibilities. In that case doing
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Responsabilities are better separated if we move it to the parent app now? I feel that the FieldAndRemove does "too much". But non blocking for me.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we have the same behavior in 2 apps right now (admin & contacts). |
||
| onRemove={onRemove} | ||
| onFocus={onFocus} | ||
| onClick={handleClick} | ||
| /> | ||
|
|
@@ -85,7 +104,7 @@ const FieldInput = ({ | |
| )} | ||
| {labelProps && ( | ||
| <HasValueCondition name={name} otherCondition={hasBeenFocused}> | ||
| <div className="u-mt-half-s u-ml-half u-ml-0-s u-flex-shrink-0 u-w-auto u-miw-4"> | ||
| <div className="u-mt-half-s u-ml-half u-ml-0-s u-flex-shrink-0 u-w-100-s u-w-auto u-miw-4"> | ||
| <Field | ||
| attributes={labelProps} | ||
| name={`${name}Label`} | ||
|
|
@@ -97,6 +116,7 @@ const FieldInput = ({ | |
| </div> | ||
| </HasValueCondition> | ||
| )} | ||
| {showRemove && !isMobile && <RemoveButton onRemove={onRemove} />} | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,18 @@ | ||||||||||||||||||||||
| import React from 'react' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| import Icon from 'cozy-ui/transpiled/react/Icon' | ||||||||||||||||||||||
| import IconButton from 'cozy-ui/transpiled/react/IconButton' | ||||||||||||||||||||||
| import CrossCircleIcon from 'cozy-ui/transpiled/react/Icons/CrossCircle' | ||||||||||||||||||||||
| import ListItemIcon from 'cozy-ui/transpiled/react/ListItemIcon' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const RemoveButton = ({ onRemove }) => { | ||||||||||||||||||||||
| return ( | ||||||||||||||||||||||
| <ListItemIcon className="u-ml-half"> | ||||||||||||||||||||||
| <IconButton aria-label="delete" size="medium" onClick={onRemove}> | ||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Localize the delete button accessibility label. Line 11 uses a hardcoded English Suggested change-const RemoveButton = ({ onRemove }) => {
+const RemoveButton = ({ onRemove, ariaLabel = 'delete' }) => {
return (
<ListItemIcon className="u-ml-half">
- <IconButton aria-label="delete" size="medium" onClick={onRemove}>
+ <IconButton aria-label={ariaLabel} size="medium" onClick={onRemove}>
<Icon icon={CrossCircleIcon} />
</IconButton>
</ListItemIcon>
)
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| <Icon icon={CrossCircleIcon} /> | ||||||||||||||||||||||
| </IconButton> | ||||||||||||||||||||||
| </ListItemIcon> | ||||||||||||||||||||||
| ) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| export default RemoveButton | ||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.