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

[UX] Make autocomplete suggestions easier to read #4481

Closed
klonos opened this issue Jul 14, 2020 · 12 comments · Fixed by backdrop/backdrop#3189
Closed

[UX] Make autocomplete suggestions easier to read #4481

klonos opened this issue Jul 14, 2020 · 12 comments · Fixed by backdrop/backdrop#3189

Comments

@klonos
Copy link
Member

klonos commented Jul 14, 2020

Makes it hard to distinguish the actual options with all the additional info:

Screen Shot 2020-07-14 at 12 38 25 pm

@klonos
Copy link
Member Author

klonos commented Jul 14, 2020

...a few options:

  • bolded suggestions:

    Screen Shot 2020-07-14 at 1 05 31 pm

  • grayed-out descriptions:

    Screen Shot 2020-07-14 at 1 08 26 pm

  • bolded suggestions + grayed-out descriptions:

    Screen Shot 2020-07-14 at 1 07 53 pm

@klonos
Copy link
Member Author

klonos commented Jul 14, 2020

I've filed a PR: backdrop/backdrop#3189

...I went with grayed-out descriptions (#666666) and also decreased their font size to 80%. Here's what it looks like:

image

@stpaultim
Copy link
Member

How widespread is this change? Are there other core autocomplete menus that would be effected by this?

On the surface, this is visually a nice improvement and I support it. I'm just trying to think of unintended sites effects or whether or not we have any conflicting styles choices elsewhere in Backdrop CMS.

@ghost
Copy link

ghost commented Jul 24, 2020

I like the grey and the smaller text, and that it matches machine name styling 👍

@klonos
Copy link
Member Author

klonos commented Jul 24, 2020

Thanks for the feedback all 🙏 ...this is ready for another round of review/testing. I've made the classes more specific, as per @quicksketch's suggestion during the dev meeting (.autocomplete-description and .autocomplete-suggestion), and also fixed the contrast of the gray description when the list items in the drop down are being selected and/or hovered-over. Before:

Screen Shot 2020-07-24 at 5 31 24 am

After:
Screen Shot 2020-07-24 at 5 44 09 am

Also made sure that the suggestion descriptions are properly positioned in RTL. Before:
Screen Shot 2020-07-24 at 5 53 24 am

After:
Screen Shot 2020-07-24 at 5 57 50 am

@ghost
Copy link

ghost commented Aug 10, 2020

@klonos I reviewed the code and made some cleanup suggestions.

@ghost ghost added this to the 1.16.3 milestone Aug 10, 2020
@klonos
Copy link
Member Author

klonos commented Aug 12, 2020

Thanks @BWPanda 👍 ...nice catch with the Views matches description; I was doing some weird variable usage in that t() there.

As for the space string concatenations, I've fixed them too, but we do not mention anything about that in https://api.backdropcms.org/php-standards#string-concatenation (nor does the respective documentation in d.org: https://www.drupal.org/docs/develop/standards/coding-standards#concat). We should either allow that, or add something about it in the coding standards documentation 😉

@ghost
Copy link

ghost commented Aug 12, 2020

As for the space string concatenations, I've fixed them too, but we do not mention anything about that in https://api.backdropcms.org/php-standards#string-concatenation

I'd have thought because it was common sense not to concatenate strings together without some sort of variable in between. Like, we wouldn't do: 'this' . ' is ' . 'some' . ' ' . 'text'...

Sorry, not having a go at you. I realise it was probably left-over from something being removed at some point. If you think it's worth adding to the standards documentation, we can do that. I just figure it's a fairly basic thing that doesn't need to be specified as such...

THanks for the changes, looks good to me. Just some test failures that I can't see details of because CI's playing up again...

@klonos
Copy link
Member Author

klonos commented Aug 12, 2020

No worries @BWPanda 😄 ...I'm not too fussed about it either. This was actually a left-over form when I had the code like this:

$match_description = '<span class="autocomplete-description">(' . t('User account') . ')</span>';
$matches[$path] = $match . ' ' . $match_description;

...I then moved the <span>s in the $matches[$path], and ended up with this:

$match_description = t('User account');
$matches[$path] = $match . ' ' . '<span class="autocomplete-description">(' . $match_description . ')</span>';

All good 👍

Re test failures, I know 😓 ...ZenCI has been giving me a hard time for some time now. I've restarted them 🤞

@klonos
Copy link
Member Author

klonos commented Aug 12, 2020

All tests are finally green 👍

@ghost
Copy link

ghost commented Aug 12, 2020

Nice! RTBC from me.

@quicksketch
Copy link
Member

Thanks @klonos and @BWPanda for your work on this! I merged backdrop/backdrop#3189 into 1.x and 1.16.x.

@jenlampton jenlampton changed the title [UX] Link autocomplete: Improve the readability/scannability of suggestions in the dropdown [UX] Make autocomplete suggestions easier to read Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants