Tags filter button colour change is not obvious

Alys commented 6 years ago

In the Trello ACCESSIBILITY SETTINGS card, S. L. Casteel wrote: "The Tags dropdown button on the Tasks page has very little contrast between the gray when no tags are selected and the purple when tags are selected. I only realized today that the color changes, and even now that I know it, I have trouble seeing which color it is. It'd be helpful to adjust one of the colors to increase the contrast."

I also hadn't noticed that the colour changed until I read this, although that might be partly because I rarely use tags anymore now that it's two clicks and scrolling to select them instead of one click. I think though the colour change would benefit from being more obvious.

Alys commented 6 years ago

As a note for anyone working on this once a new design has been provided, testing should be done with a browser extension that mimics the various kinds of colour-blindness to check that the difference is visible to anyone, for accessibility reasons. There's a lot of extensions to choose from and all the ones I've used in the past have been easy to use: https://www.google.com.au/search?q=browser+extension+colour+blind

Tressley commented 5 years ago

I've increased the contrast on the border-color, font-color, icon and arrow color, in addition to adding a new "Clear tags" component that will clear the current filter query.

benkelaar commented 4 years ago

I'd like to fix this, but I'm myself quite colourblind so even with browser extensions I will not be able to validate all colour combinations.

However the original description suggests that the colour change just needs to be made more pronounced, while currently (even when I look at the css) there's no colour-change at all.

Should I add this colour (#6133b3, right, @Tressley ?) to colors.scss? What name should it get?

@Tressley What's the colour of "Clear tags", is that #45a0d1?

Alys commented 4 years ago

I've set this back to "on hold - needs design" since a design-related decision is needed from @benkelaar's questions for @Tressley

Tressley commented 4 years ago

Thanks for your interest in this @benkelaar! We're actually tackling the button states internally, so we don't need to worry about it in this issue. They should be out in the wild soon. However, if you're interested in implementing the "Clear tags" feature, we can tackle it in this issue. Is that something you're interested in?

If so, the "Clear tags" color is just the base link color for Habitica, #2995CD (or blue-10) that should underline on hover.

benkelaar commented 4 years ago

Sounds good, I'll look at implementing the clear tags note.

What I was thinking, since it lives on the filter bar itself, wouldn't it make more sense to have a "Clear filters" link instead (i.e. also remove the text when you hit it)?

Tressley commented 4 years ago

@benkelaar -- No, we should keep the "Tags" text as that's what the related control is called. I have a request though -- It looks like the "Clear all filters" text still exists within the dropdown. Would you be willing to update that to say "Clear selection"?

There are also spacing updates that need to be made to this dropdown as well. If you're interested in tackling those, we can contain them in this issue and I'll get a design into Zeplin for you. 👍

benkelaar commented 4 years ago

@Tressley Ah yeah, I remember that it was that "Clear all filters" label that actually caused me to think that that is the way it should work.

How does it work for translations, should I just remove all translations to indicate that the label needs to be re-translated?

Definitely willing to tackle spacing etc. in this tag popup, will you also redesign the editable version or does that remain as is?

Tressley commented 4 years ago

@benkelaar -- I'm actually not sure how to handle the translations. Let me tag in @paglias to see if he has any guidance for when translated text is changed and deviates from the original.

As for the states, I'll account for the edit state as well. Hopefully not too many changes outside of spacing and maybe some colors and component states.

paglias commented 4 years ago

@benkelaar for the translation:

If the string is only used there just change the English version
If it's used elsewhere and we want to keep the old string in the other places you can add a new string

benkelaar commented 3 years ago

Apologies for the long absence, I'll pick this up again.

benkelaar commented 3 years ago

@paglias In order to make sense of the user.vue component, I abstracted the filter segment into a separate component and extracted a separate component for tag filtering out of that.

I refrained from doing other refactorings for now, since I wanted to highlight the changes that went into setting up these components.

In essence this merge request can be merged as is, but I marked it WIP so that I can also do the refactoring, unit tests and actual fix for the issue in there, but I can also do those in separate merge requests if you prefer.

Tressley commented 3 years ago

@benkelaar -- Similarly, I let this fall off for a bit. I did manage to get some updates in which can be found here: https://zpl.io/adlyOqe

benkelaar commented 3 years ago

@Tressley Nice, those already look much better. Will work well with some of the refactorings I was thinking about as well.

benkelaar commented 3 years ago

I think I've gotten prettty close:

Nothing selected Note: I just noticed that there's too much space between the edit & clear links, fixed that now.

With selection

Editable

With the refactorings I did I did lose the markdown capability for the tags, but I'll re-add that maybe tonight or tomorrow.

@Tressley I have a couple of questions:

You've shown the icons on the first challenge tag, I've assumed that they only show up on hover like what happens with the current remove icon as well. However, the challenge order is currently not adjustable, so I do not show the drag handle on challenge tag, is that as it should be? With the new refactorings, I think it should be pretty trivial to implement sort ordering of challenge tags as well BTW.
What should happen when hovering over the Save button? (Currently it's a fancy link and it gets underlined, but since it looks kinda like a button you may want some "buttony" hover behaviour)
How do you want long tags handled? Currently they're cut off in edit mode but expand to the next line in selection mode, do you want to retain that behaviour?
There's currently a "make bottom border purple on focus" rule, I've kept that in.
You haven't shown the "New tag" field, I've kept it reasonably similar to what it is now.
Edit: One more - Would it make sense to make the "Clear tags" link only visible when there is a tag selection?

@paglias I've basically refactored the complexity of editing vs selecting away from the HTML and into the SCSS. I think that the elements are basically the same and the difference is more a display issue, which I think can be more intuitively solved with CSS.

There is quite a bit of CSS however, so what I want to do (while re-introducing tag markdown) is extract a separate component for a single tag display. I think that'll make it quite a bit easier to grok as a whole since that'll separate the tag-states from the filter states.

About that CSS, I usually prefer to use semantic HTML and then identify components with paths over adding classes just to make layouting work, but I do recognize that classes can make both CSS and HTML more expressive. Is there a standard way this is approached in the Habitica codebase?

paglias commented 3 years ago

@benkelaar extracting the single tag display in a separate component is great!

As for the css we usually do scoped classes but if it's just targeting markdown elements and the css selectors are easy to understand I think it'll be okay

benkelaar commented 3 years ago

...and the css selectors are easy to understand...

Yeah I think I've kinda landed on the wrong side of this equation in some cases... I'll have a look at if I can simplify it with some sensible classes (I think an extra component will help here as well).

benkelaar commented 3 years ago

@paglias What's your position on applying css per ID selector? To me if you have specific styling for a single element using an ID seems to make more sense than a class, but I don't see that happen often. Do you know why?

benkelaar commented 3 years ago

...and the css selectors are easy to understand...

Yeah I think I've kinda landed on the wrong side of this equation in some cases... I'll have a look at if I can simplify it with some sensible classes (I think an extra component will help here as well).

I think all these problems are now pretty much resolved. Quite happy with how it turned out with the new tag-button component.

Tressley commented 3 years ago

@benkelaar -- Thanks for your work on this so far. Things are really looking good. Let me see if I can answer some of your questions.

You've shown the icons on the first challenge tag, I've assumed that they only show up on hover like what happens with the current remove icon as well.

Yes, both icons should only be displayed on hover.

However, the challenge order is currently not adjustable, so I do not show the drag handle on challenge tag, is that as it should be?

Challenge tags should be sortable, but not editable.

What should happen when hovering over the Save button?

This should actually be a button and use our button styles and states.

How do you want long tags handled? Currently they're cut off in edit mode but expand to the next line in selection mode, do you want to retain that behaviour?

While they're not the prettiest, I think both of these are fine. Cutting off in edit mode prevents wrapping and selection mode should have the full string displayed.

You haven't shown the "New tag" field, I've kept it reasonably similar to what it is now.

Ah, good catch! I think keeping it as close to the current version is fine for now. 👍

Would it make sense to make the "Clear tags" link only visible when there is a tag selection?

Just to clarify, this is the clear component ON the Task page, not in the dropdown, correct? If so, yes, that should only be displayed when there is a selection.

benkelaar commented 3 years ago

Challenge tags should be sortable, but not editable.

Are you sure? Because I believe the current behaviour is editable but not sortable. I agree that your version seems to make more sense though, so I'm fine with implementing it that way.

All other answers seem fine and I can work with, thanks a lot!

Tressley commented 3 years ago

You're right! That was my mistake, I was caught in two minds with groups. Group tags aren't editable, but Challenge tags are.

SabreCat commented 3 years ago

@benkelaar appears to have moved on from this, but his PR shouldn't need too much further work to wrap up if anyone is interested!

HabitRPG / habitica

Tags filter button colour change is not obvious #9845