@diosmosis opened this Pull Request on November 25th 2018 Member

… & add missing translation.

image

Changes:

  • In select form-fields', if there is already an '' empty option, use that in the dropdown. This still allows showing the placeholder if the value is set to, eg, null or undefined).
  • Add a 'Show all' entry to the access level filter dropdowns.
  • Add a missing translation.

Fixes #13698

@Findus23 what do you think? Would this make selecting all sites clearer to you?

The placeholder font color is theme related so won't address in this issue. Though if it's a problem, perhaps a separate issue is called for.

@tsteur commented on November 26th 2018 Member

fyi it worked for me and code looks 👍 .

From a UX point of view it might be worth thinking about having some content above this saying something like "Give access to all websites [permission selector] [confirm button]". Then we could also add a note that user will get only access to currently existing websites, not to websites created in the future. But we could always add this later in a later release as it doesn't seem too important maybe.

@diosmosis commented on December 9th 2018 Member

@tsteur added the addition you suggested to the component

@tsteur commented on December 9th 2018 Member

Works :+1: and nice to use. feel free to merge once tests pass

@Findus23 commented on December 9th 2018 Member

@diosmosis Thanks for the fix.

This Pull Request was closed on December 10th 2018
Powered by GitHub Issue Mirror