Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

[accessibilité des inputs] - Amélioreration des états read-only et waiting des inputs #233

Closed
wants to merge 22 commits into from
Closed

[accessibilité des inputs] - Amélioreration des états read-only et waiting des inputs #233

wants to merge 22 commits into from

Conversation

raphpare
Copy link
Member

Copie du PR 202

Description

Description du bogue dans ce billet: #150
Corrige également ce billet: #80

Types de changements

  • Correction de bug (sans breaking change)
  • Amélioration (ajout par example une nouvelle propriété, évènement, slot ou méthode à un composant existant sans breaking change)
  • Nouvelle fonctionalité (nouveau composant, directive, filtre ou service)
  • Breaking change (modification à une fonctionnalités existante qui nécessite une migration remplir la section release note)
  • Refactoring/ménage (sans breaking change)
  • Documentation/storybook (changement à la documentation ou aux storybooks qui n'affecte aucun package)
  • Autre

Comment cela peut-il être testé?

  • Test unitaire (un nouveau test unitaire à été fait)
  • Storybook
  • Test manuel / Sandboxes
  • Autre

Liste des composant avec une nouvelle storie qui se nomme "state"

  • m-textfield
  • m-textarea
  • m-datepicker
  • m-timepiker
  • m-dropdown
  • m-select
  • m-multi-select
  • m-typeahead
  • m-autocomplete
  • m-rich-text-editor
  • m-decimalfield
  • m-moneyfield

Inclure cette section dans les release notes

Changes in the states of the input components

  • Read-only
    • Read-only items can receive focus.
    • Value of readonly items can be copy.
    • Read-only items are part of the navigation sequence with the key.
    • Read-only items have the same visual as deactivated items. However, the value text is not grayed out.
    • Readonly items aren't disabled.
  • Waiting
    • Waiting items can receive focus.
    • Value of waiting items can be edit, copy and delete as standard input component.
    • Waiting items are part of the navigation sequence with the key.
    • Waiting items are neither disabled nor read-only by default, but can be disabled if necessary by activating the disabled and / or readonly porps.

Component impacted by these modifications

  • m-textfield
  • m-textarea
  • m-datepicker
  • m-timepiker
  • m-dropdown
  • m-select
  • m-multi-select
  • m-typeahead
  • m-autocomplete
  • m-rich-text-editor
  • m-decimalfield
  • m-moneyfield

Liens internes

#150
#80

@raphpare raphpare requested a review from jpguilmette February 17, 2020 15:07
@raphpare raphpare added the enhancement non-breaking change which adds functionality such as new props, slots, event or method to service label Feb 17, 2020
This was linked to issues Feb 17, 2020
bogue: Identifier l'état focus d'un élément readonly #80
Open
bogue: les éléments interatifs sont désactivés (disabled) en lecture seule, et perdent le focus #150
Open
@chuckmah chuckmah changed the title Améliorer états des champs V2 [accessibilité des inputs] - Amélioreration des états read-only et waiting des inputs Feb 17, 2020
@raphpare raphpare added the next When a PR should be merged in the next version (minor or major) label Feb 17, 2020
@chuckmah chuckmah requested a review from jipigi February 17, 2020 16:31
@chuckmah chuckmah added this to the 1.1.0 milestone Feb 17, 2020
@chuckmah chuckmah changed the base branch from master to develop February 17, 2020 18:16
chuckmah
chuckmah suggested changes Feb 17, 2020
View reviewed changes
Copy link
Contributor

@chuckmah chuckmah left a comment

Choose a reason for hiding this comment

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

Svp voir pour les regression au niveau du m-phonefield

src/storybook/src/modul-components/components/textfield/textfield.stories.ts
@@ -61,19 +59,83 @@ storiesOf(`${modulComponentsHierarchyRootSeparator}${TEXTFIELD_NAME}`, module)
template: '<m-textfield label="Label" :required-marker="true"></m-textfield>'
}))
.add('waiting', () => ({
template: '<m-textfield :waiting="true"></m-textfield>'
template: `<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

il faudrait mettre seulement un textfield par story et utiliser le bon nom qui documente qu'est que la story test. Sinon c'est difficile de voir qu'est que la story teste

par example waitingReadOnly waitingDisabled

Copy link
Contributor

Choose a reason for hiding this comment

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

Ou sinon tu peux utiliser label pour documenter , mais est-ce que c'est story est redondante avec la story général "state" ?

packages/modul-components/src/components/select/base-select/base-select.html Show resolved Hide resolved
packages/modul-components/src/components/select/base-select/base-select.scss Show resolved Hide resolved
@chuckmah
Copy link
Contributor

@jipigi @vidal7

Il faudrait voir les impacts qu'un tel changement peut apporter dans Brio , Avec ce changement les champs waiting=true seront maintenant éditable mais on peut facilement revenir a l'ancien comportement en ajoutant un disabled=true lorsque waiting=true.

Je le ferai dans 1.1 si possible sinon on peut postponner a 2.0 mais la PR risque de ne pas être mise a jour.

@chuckmah chuckmah requested a review from vidal7 February 17, 2020 19:05
@vidal7
Copy link
Member

vidal7 commented Feb 18, 2020

@jipigi @vidal7

Il faudrait voir les impacts qu'un tel changement peut apporter dans Brio , Avec ce changement les champs waiting=true seront maintenant éditable mais on peut facilement revenir a l'ancien comportement en ajoutant un disabled=true lorsque waiting=true.

Je le ferai dans 1.1 si possible sinon on peut postponner a 2.0 mais la PR risque de ne pas être mise a jour.

@jipigi, est ce que ce comportement est souhaitable par défaut dans Brio?

@vidal7
Copy link
Member

vidal7 commented Feb 18, 2020

Je trouve ça lourd comme changement et c'est dur d'évaluer les impacts dans les projets tels que Brio. Je me demande si nous aurions pas pu faire ce changement sans breaking change sous un flag en permettant de faire un "opt-in" pour tester. Là, essentiellement, cela impacte tous les inputs de modUL et à cause de cette complexité, je ne peux pas mesurer l'impact sur Brio ni approuver cette PR.

@chuckmah
Copy link
Contributor

Je trouve ça lourd comme changement et c'est dur d'évaluer les impacts dans les projets tels que Brio. Je me demande si nous aurions pas pu faire ce changement sans breaking change sous un flag en permettant de faire un "opt-in" pour tester. Là, essentiellement, cela impacte tous les inputs de modUL et à cause de cette complexité, je ne peux pas mesurer l'impact sur Brio ni approuver cette PR.

Merci @vidal7, concernant la stratégie je pensais merger cette PR en develop (avec PR #161 et #229) et ensuite faire une version 1.1.0-beta.1 qui devrait être testé directement dans les projets (brio et AEL). Ensuite après le feedback du QA on pourrait effectuer les changements à savoir si on doit corriger d'autres bugs ou rollbacker complètement la PR. Ensuite quand tout le monde est content je peut faire la release officiel de la 1.1.0

C'est la stratégie la plus simple que je vois (qui est basé pas mal sur ce qui se fait dans un projet open source) qui nous permettrais de faire des évolutions général comme celle décrite dans la PR.

Aussi la demande d'évolution provient de @jipigi, il faudrait voir a quel point cette amélioration est souhaitable et sinon comment on aurait pu la découper autrement

@chuckmah chuckmah removed this from the 1.1.0 milestone Mar 19, 2020
@chuckmah
Copy link
Contributor

@raphpare merci pour les changements, vu l'ampleur des changements nous allons attendre les tests de @jipigi avant de l'inclure dans une version mineur de modul

@raphpare raphpare closed this Nov 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@chuckmah chuckmah chuckmah approved these changes

@guignol1981 guignol1981 Awaiting requested review from guignol1981 guignol1981 is a code owner

@jpguilmette jpguilmette Awaiting requested review from jpguilmette

@jipigi jipigi Awaiting requested review from jipigi

@vidal7 vidal7 Awaiting requested review from vidal7

Assignees
No one assigned
Labels
enhancement non-breaking change which adds functionality such as new props, slots, event or method to service next When a PR should be merged in the next version (minor or major)
Projects
None yet
Milestone
No milestone
3 participants
@raphpare @chuckmah @vidal7