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

enh: enlarge search icon for better visibility #8459

Merged
merged 1 commit into from Nov 9, 2020

Conversation

kushthedude
Copy link
Member

@kushthedude kushthedude commented Nov 8, 2020

Signed-off-by: Kush Trivedi kushthedude@gmail.com

[ ] Configuration Infrastructure
[X] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[X] User Experience
[ ] Developer Infrastructure

Signed-off-by: Kush Trivedi <kushthedude@gmail.com>
@kushthedude kushthedude requested a review from a team as a code owner November 8, 2020 05:10
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Nov 8, 2020
@istio-testing istio-testing added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 8, 2020
@istio-testing
Copy link
Contributor

Hi @kushthedude. Thanks for your PR.

I'm waiting for a istio member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Member

@sdake sdake left a comment

Choose a reason for hiding this comment

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

I am not sure what the first file changed does. I am APPROVING this PR - with the assumption that another reviewer will ensure the changes to the layout file are correct.

Really nice job - especially for a first PR!!

Rendering: https://deploy-preview-8459--preliminary-istio.netlify.app/

For a followup independent PR, would you be willing to enhance the size of the Options and Settings gear icon to match the size of this search icon?

And for a final PR - could you enhance the size of the X box next to the search icon. It is really hard to find...

It may be that not all of these enhancements are accepted, which is why I requested per PR enhancements (so they could be individually reviewed).

I find these types of changes super beneficial for those with vision difficulty (such as myself). These relatively small changes have an awesome impact on accessibility.

@sdake
Copy link
Member

sdake commented Nov 8, 2020

/ok-to-test

@sdake sdake added ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. and removed needs-ok-to-test labels Nov 8, 2020
@kushthedude
Copy link
Member Author

Thanks, @sdake!

I am not sure what the first file changed does.

The first file change is needed to add an extra attribute to the class with the icon name so that when the class name would be icon magnifier both svg.icon && svg.magnifier styles will be applied.

It may be that not all of these enhancements are accepted, which is why I requested per PR enhancements (so they could be individually reviewed).

Sure I will make the follow-up PRs for the issues you mentioned 😄

@frankbu
Copy link
Collaborator

frankbu commented Nov 9, 2020

@kushthedude Thanks for the enhancement, but I'm not sure if we should be tweaking these kinds of things outside of a more coordinated overall improvement to the site usability. There is work underway to improve the UX and layout, but maybe this is OK as a temporary improvement?

@craigbox, how does this fit with things you are planning?

@craigbox
Copy link
Contributor

craigbox commented Nov 9, 2020

We'll be doing a complete visual overhaul of the site in the next 3 months, but it will probably not land for a while, and so enhancements like these to the existing site are super useful. Thanks, Kush!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/user experience cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. kind/docs ok-to-test Set this label allow normal testing to take place for a PR not submitted by an Istio org member. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants