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

FileStatusList: indicate active filter #6449

Closed
mstv opened this issue Apr 7, 2019 · 11 comments · Fixed by #6470
Closed

FileStatusList: indicate active filter #6449

mstv opened this issue Apr 7, 2019 · 11 comments · Fixed by #6470

Comments

@mstv
Copy link
Member

mstv commented Apr 7, 2019

Feature description

An entered file filter for the FileStatusList can be overlooked too easily.
The ComboBox's window color (or text color for a future dark theme) should be changed if the filter string is not empty.
Maybe we could even indicate the validity of the regex, e.g. green and red.

In preparation of switching color schemes, a new Control class should be created.

Thoughts?

Before

old

After

regex valid
ok

regex error
error

Environment

  • Git Extensions 3.1.0
  • Build 28fcbf1
  • Git 2.20.1.windows.1
  • Microsoft Windows NT 6.1.7601 Service Pack 1
  • .NET Framework 4.7.2117.0
  • DPI 96dpi (no scaling)
@RussKie
Copy link
Member

RussKie commented Apr 7, 2019

Agree that the fact that a filter is active could be made more prominent. 👍

Not particularly comfortable with the choice of a colour though.

What is a given filter doesn't yield any results? Green colour is an indicator of success/permission (e.g. green tick, green colour of traffic lights).
If the filter yielded no results the filter box should perhaps be red or orange (i.e. a warning colour).

I'm also not sure about setting the background colour of the filter control, it feels overpowering.
Perhaps colouring the control's borders to a respective colour could be a solution.

Require further experimentations.

@RussKie RussKie added area: user experience up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/ type: feature request labels Apr 7, 2019
@gerhardol
Copy link
Member

I would prefer to just have the filter text in bold.
The green is ugly, I would vote against a PR with that color...

@mstv
Copy link
Member Author

mstv commented Apr 7, 2019

That's exactly how Wireshark handles it.
I knew you would not like it very much. That's why I haven't implemented it yet.

What is a given filter doesn't yield any results? Green colour is an indicator of success/permission (e.g. green tick, green colour of traffic lights).
If the filter yielded no results the filter box should perhaps be red or orange (i.e. a warning colour).

This would be over-engineered in my opinion.

Perhaps colouring the control's borders to a respective colour could be a solution.

Might look good. Tough much too much effort.

I would prefer to just have the filter text in bold.

Good idea, very good compromise.

@ghost
Copy link

ghost commented Apr 7, 2019

What about using translucent pink/reddish pink or translucent orange as the warning color? Would look less offensive at the same time as highlighting something that needs attention.

@gerhardol
Copy link
Member

Having separate colors or styles are too much in my opinion, I vote against it.

@ghost
Copy link

ghost commented Apr 7, 2019

Fair enough, although translucent pink/reddish pink is already present in diff view and the color of the local branch tag in the commit list.

@RussKie
Copy link
Member

RussKie commented Apr 8, 2019 via email

@mstv
Copy link
Member Author

mstv commented Apr 11, 2019

I've updated the feature description with new faked screenshots with bold text and the colors from the diff view.

In an other recent review, it was requested to not use hard coded colors.
One could create a class ControlColorer that sets the ForeColor and the BackColor of a Control in dependency on a state.
Do you prefer a different approach or know a better name?

@ghost
Copy link

ghost commented Apr 12, 2019

Color selection looks good to me.
No opinion on the names, sorry.

@RussKie
Copy link
Member

RussKie commented Apr 13, 2019

I don't mind the colours. I presume once the filter is removed, the background is reset to the standard colour.

@RussKie
Copy link
Member

RussKie commented Apr 13, 2019

One could create a class ControlColorer that sets the ForeColor and the BackColor of a Control in dependency on a state.
Do you prefer a different approach or know a better name?

I'd think we should have a "theme" or "style" class that contains all the colours we use.

@RussKie RussKie removed the up-for-grabs Easy tasks for those looking to get involved. Refer to https://up-for-grabs.net/ label Apr 18, 2019
@RussKie RussKie added this to the 3.1.0 milestone Apr 19, 2019
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