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

Log viewer: Filter toggle #6863

Merged
merged 5 commits into from
Oct 29, 2019
Merged

Log viewer: Filter toggle #6863

merged 5 commits into from
Oct 29, 2019

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Oct 25, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

Description

I have improved the filter dropdown, converting the <a> to <button> and adding appropiate aria attributes as well.

I have also added some empty alt attributes on the images rendering the favicons for different searches. This is done in order to let screen readers know they can safely ignore the image since the provided text is adequate in a non-visual context.

Furthermore I have modified some styling so the control aligns with the left edge and looks better in general.

There is more stuff to be done in this view but I'm going to split it all up in smaller PR's since doing one big file, would be confusing for everyone involved 😃

Before
log-filter-before

After
log-filter-after

@nul800sebastiaan nul800sebastiaan merged commit c7af359 into umbraco:v8/dev Oct 29, 2019
@nul800sebastiaan
Copy link
Member

Thanks!

FYI This one got a lot trickier to merge @BatJan due to the search.html now having translations and this PR not having those yet, so I had to be careful. Think I managed though 🤞

@BatJan
Copy link
Contributor Author

BatJan commented Oct 30, 2019

@nul800sebastiaan Ah, sorry about that - Tried doing the thing where I did more minor PR's to make the review process easier... turns out it made the merging experience worse... 🤷‍♂ 😁

I'm confident you managed to get it right though 😃

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

Successfully merging this pull request may close these issues.

4 participants