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

Add chevron to dropdowns that are missing them #6242

Merged
merged 2 commits into from Jul 21, 2020

Conversation

rinti
Copy link
Contributor

@rinti rinti commented Jul 19, 2020

This is a fix for #6072

I reused already existing markup for dropdowns, however I needed to make a small addition to the css for the site dropdown. There might be a better way to do it so the addition isn't needed - if so I'm all ears. :)

I also added a translation tag for "Site", since I was already in the file and noticed, and it felt to minor to open a pull request for separately.

Site settings before:
Screenshot 2020-05-26 at 14 03 06

Site settings after:
Screenshot 2020-07-19 at 20 38 14

Adding documents before:
Screenshot 2020-07-19 at 19 57 02

Adding documents after:
Screenshot 2020-07-19 at 20 38 52

Adding images:
Screenshot 2020-07-19 at 20 00 13

Adding images after:
Screenshot 2020-07-19 at 20 39 27

@squash-labs
Copy link

squash-labs bot commented Jul 19, 2020

Manage this branch in Squash

Test this branch here: https://rintifix-dropdowns-ldevx.squash.io

@thibaudcolas thibaudcolas self-requested a review July 20, 2020 11:46
@thibaudcolas thibaudcolas added this to In progress in WCAG2.1 & ATAG 2.0 for CMS via automation Jul 20, 2020
@thibaudcolas thibaudcolas added this to the some-day milestone Jul 20, 2020
@thibaudcolas thibaudcolas linked an issue Jul 20, 2020 that may be closed by this pull request
@thibaudcolas
Copy link
Member

Thank you @rinti, I’ll try to have a look as soon as I can.

Copy link
Member

@thibaudcolas thibaudcolas left a comment

Choose a reason for hiding this comment

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

👍 tested in Firefox, Chrome, IE11, all looking as expected. Thank you @rinti!

{% endfor %}
</select>
<span></span>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

This markup looks pretty weird but I can see it’s a carbon copy of collection_chooser.html so 👍 . Thank you for digging for this.

{% endfor %}
</select>
<span></span>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

Same as above, 👍 .

<div class="field choice_field">
<form method="get" class="setting-site-switch-form" id="settings-site-switch" novalidate>
<label for="{{ site_switcher.site.id_for_label }}">
{% trans "Site" %}:
Copy link
Member

Choose a reason for hiding this comment

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

Thank you 🙏 this feels like as big of a fix as the dropdown arrows missing.

// the site setting dropdown is auto width, so the chevron will overlap with text if not padded
.choice_field .setting-site-switch-form .input select {
padding-right: 5em;
}
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice for this to be defined in the stylesheets for that contrib app only, but I see there are no stylesheets for it right now, so that also feels overkill.

I think I’m happy with this as-is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think that this perhaps should be the default for all the choosers instead, I didn't think to check but I think this problem exists for all choosers - if the choice text is long enough.

@thibaudcolas thibaudcolas modified the milestones: some-day, 2.10 Jul 21, 2020
@thibaudcolas thibaudcolas merged commit f9c5bab into wagtail:master Jul 21, 2020
WCAG2.1 & ATAG 2.0 for CMS automation moved this from In progress to Done Jul 21, 2020
Accessibility team actions automation moved this from To review to Done Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Accessibility issue for dropdowns in Wagtail Admin
2 participants