Skip to content
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

[R2] Form #163

Open
wants to merge 28 commits into
base: develop
Choose a base branch
from
Open

[R2] Form #163

wants to merge 28 commits into from

Conversation

@github-actions
Copy link

👋 @g-stamatis

To generate the Storybook and Docusaurus websites, write comment with:

@eea-jenkins build all

To generate only Storybook write comment with:

@eea-jenkins build storybook

To generate only Docusaurus write comment with:

@eea-jenkins build docusaurus

@g-stamatis
Copy link
Contributor Author

@eea-jenkins build all

@jenkins-ci-eionet-europa-eu
Copy link

@ichim-david
Copy link
Member

@g-stamatis you still have this problem where the border changes color when the dropdown is visible and the cursor is outside of the dropdown area
outside-hover-border-color-change

@ichim-david
Copy link
Member

border-dropdown
here is the code that adds the green border

@g-stamatis
Copy link
Contributor Author

@eea-jenkins build storybook

@jenkins-ci-eionet-europa-eu
Copy link

❌ Storybook build FAILED

Check https://ci.eionet.europa.eu/job/volto-addons/job/volto-eea-design-system/job/PR-163/25/ for details

🔥 @g-stamatis

@jenkins-ci-eionet-europa-eu
Copy link

@tarantilis
Copy link
Collaborator

Updates to fix accessibility issues. The list of changes will be logged to the form ticket:
https://taskman.eionet.europa.eu/issues/147795

More updates or improvements (if needed) will be included in a new PR along with an example of a full form.

@tarantilis
Copy link
Collaborator

@eea-jenkins build storybook

@jenkins-ci-eionet-europa-eu
Copy link

❌ Storybook build FAILED

Check https://ci.eionet.europa.eu/job/volto-addons/job/volto-eea-design-system/job/PR-163/41/ for details

🔥 @tarantilis

@tarantilis
Copy link
Collaborator

@eea-jenkins build storybook

@jenkins-ci-eionet-europa-eu
Copy link

@g-stamatis
Copy link
Contributor Author

@eea-jenkins build storybook

@jenkins-ci-eionet-europa-eu
Copy link

@tarantilis
Copy link
Collaborator

@eea-jenkins build storybook

@jenkins-ci-eionet-europa-eu
Copy link

@tarantilis tarantilis requested review from ichim-david and removed request for ichim-david September 20, 2022 10:03
@tarantilis
Copy link
Collaborator

After testing the message component looks unaffected by this PR commits and merges.
If the form error messages need some refactoring, we can update them in a new PR that will contain the full form example (preferably).

@avoinea avoinea removed their request for review October 27, 2022 13:36
@@ -89,15 +89,15 @@ function CheckboxForm({
) : null}
<Form.Field error={invalid}>
<Checkbox label="Option 1" defaultChecked id="field1"></Checkbox>
<label htmlFor="field1">Helper text for option 1</label>
<p class="description">Helper text for option 1</p>
Copy link
Member

Choose a reason for hiding this comment

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

@g-stamatis Hmm. I'm not sure about this change.

checkboxes, radio, text inputs should have the label with the for attribute present.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@avoinea The semantic uses the label prop inside the Checkbox component (line 91) so we had accesibility warnings for adding another label for the helper text. I don't clearly remember but I think we talked in a daily meeting and decided to just put a description p.

Copy link
Member

@ichim-david ichim-david left a comment

Choose a reason for hiding this comment

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

https://62713c57bcbdf4004a3d99b8-oxzzvhflft.chromatic.com/?path=/story/components-forms-dropdown--error-dropdown&args=required:true;search:true
There are still accessibility errors as seen while testing
accessibility-error

This is more serious than that mentioned by Alin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants