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

[Maps] Improving design and a11y for attribution layer settings #38

Merged
merged 4 commits into from
Apr 30, 2021

Conversation

elizabetdev
Copy link

Summary

a11y changes

  • We're using a <fieldset /> to signal that we're going to have a group of related form fields
  • We removed the EuiFormRow because it was not doing anything. And then once the popover was open the label would be focused and it seemed a bug.
  • The title is a legend for that <fieldset />.
  • Added missing aria-label for buttons.

Design

  • We added a EuiHorizontalRule so that we can clearly see that the attribution is a different section.
  • The attribution lives inside a panel and the edit and clear buttons are bottom right-aligned so that they seem they control what is inside that panel.

attribution-flow@2x

Copy link
Owner

@nreese nreese left a comment

Choose a reason for hiding this comment

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

Thanks for putting together a new design for the the attribution.

Would it be possible to remove the horizontal line and keep the columnCompressed form row? The new design makes the attribution really stand out while its no more important then any of the other rows.

@@ -1,3 +1,3 @@
.mapAttributionPopover {
width: $euiSizeXXL * 12;
width: $euiSizeXXL * 8;
Copy link
Owner

Choose a reason for hiding this comment

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

Can we keep the original size? URLs can get very long and having more horizontal space makes the text input more usable.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I'll try to add some styles to make it more similar to the columnCompressed form row. I'll also try to remove the horizontal line.

@elizabetdev
Copy link
Author

@nreese here's the new design updates based on your feedback:

attribution-flow-v2@2x

@nreese
Copy link
Owner

nreese commented Apr 30, 2021

@miukimiu That looks fantastic. Thank you

Copy link
Owner

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM

@elizabetdev
Copy link
Author

@nreese can you merge it?

Only those with write access to this repository can merge pull requests.

@nreese nreese merged commit 3e3f2ab into nreese:attribution Apr 30, 2021
@nreese
Copy link
Owner

nreese commented Apr 30, 2021

I have merged the changes, thanks

nreese added a commit that referenced this pull request May 3, 2021
* [Maps] add attribution to layer editor

* update sources to getAttributionProvider

* remove attribution UI from EMS_XYZ source

* remove attribution UI from WMS source editor

* clean up

* tslint

* AttributionFormRow jest test

* add migration

* tslint

* i18n fixes

* attribution

* [Maps] Improving design and a11y for attribution layer settings (#38)

* Design and a11y improvements

* Buttons aria labels

* Addressing PR review

* tslint and i18n fixes

* update jest snapshots

* remove placeholder for url

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Elizabet Oliveira <[email protected]>
nreese added a commit that referenced this pull request May 4, 2021
* [Maps] add attribution to layer editor

* update sources to getAttributionProvider

* remove attribution UI from EMS_XYZ source

* remove attribution UI from WMS source editor

* clean up

* tslint

* AttributionFormRow jest test

* add migration

* tslint

* i18n fixes

* attribution

* [Maps] Improving design and a11y for attribution layer settings (#38)

* Design and a11y improvements

* Buttons aria labels

* Addressing PR review

* tslint and i18n fixes

* update jest snapshots

* remove placeholder for url

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Elizabet Oliveira <[email protected]>
nreese added a commit that referenced this pull request May 5, 2021
* [Maps] add attribution to layer editor

* update sources to getAttributionProvider

* remove attribution UI from EMS_XYZ source

* remove attribution UI from WMS source editor

* clean up

* tslint

* AttributionFormRow jest test

* add migration

* tslint

* i18n fixes

* attribution

* [Maps] Improving design and a11y for attribution layer settings (#38)

* Design and a11y improvements

* Buttons aria labels

* Addressing PR review

* tslint and i18n fixes

* update jest snapshots

* remove placeholder for url

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Elizabet Oliveira <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Elizabet Oliveira <[email protected]>
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.

2 participants