UI: default properties look too similar to properties set to an empty string #1444

Closed
ericwa opened this Issue Sep 19, 2016 · 12 comments

Comments

Projects
None yet
2 participants
Collaborator

ericwa commented Sep 19, 2016

It's hard to see that "style" is set to the empty string here:
screen shot 2016-09-19 at 3 27 09 pm

I'd suggest drawing the default property keys in gray text, or maybe make the keys that the user set bold.

Here is quark for comparison, the icons make it easier to see which keys are defaults IMO:
screen shot 2016-09-19 at 3 33 47 pm

kduske added this to the TrenchBroom 2.0.0 milestone Sep 20, 2016

@kduske kduske added a commit that referenced this issue Sep 25, 2016

@kduske kduske #1444: Make default attributes better visible. Remove visual indicati…
…on of subset attributes.
bd0e3e5
Owner

kduske commented Sep 25, 2016

@ericwa could you have a look at branch feature/1444 and let me know how that works?

Collaborator

ericwa commented Sep 25, 2016

That looks great, thanks!
screen shot 2016-09-25 at 11 33 51 am

Owner

kduske commented Sep 25, 2016

Great. While we're at it: When you select multiple entities, the table will show the union of all the entities' properties. Do you think we should visually set those properties apart that only a subset of the entities has from those properties that all the entities have? The reason is that changing one of the former will add that property to all selected entities that didn't have it before.

Collaborator

ericwa commented Sep 25, 2016

Yeah, that would be good.

I just checked Quark. Properties in a multiple selection which are set on only a subset of entities have the icon greyed out, which is the same visual indication that it uses for default properties (but default properties aren't shown in Quark when you have a multiple selection.)

screen shot 2016-09-25 at 12 49 15 pm
^ both entities have "angle" "360", the light is the only one with "light" set.

Also, showing some indication like "- differs -" would be good, currently TB just shows a blank when the entities in the selection have different values:
screen shot 2016-09-25 at 12 53 36 pm

Owner

kduske commented Sep 25, 2016

Agree on both counts, and I have set the colour of property subsets to light gray.

As for showing a placeholder text (I prefer multi over differs, the problem with that is that it's fake. When you click on the property to edit it, the initial value will be multi, which I dislike. I would prefer if the initial value was empty when you start editing. This would probably be possible with a custom cell editor, but to be honest I'm too lazy for that right now.

I have implemented the faked version in the last commit. What do you think? Maybe it's still better than having the field blank all the time.

Collaborator

ericwa commented Sep 25, 2016

Nice. Just tried it, I think the fake implementation for "multi" is better than a blank field.

My only remaining nitpick is that selecting a row hides the text color. I have two entities selected, with only one having the "light" key set:
screen shot 2016-09-25 at 1 58 47 pm
But if that row is selected, it looks like both entities have it set:
screen shot 2016-09-25 at 1 58 32 pm
screen shot 2016-09-25 at 1 56 49 pm

I'm not sure what to suggest, other than using bold like Xcode does. Could make the property editor look more visually busy, though.
screen shot 2016-09-25 at 2 02 01 pm

Owner

kduske commented Sep 25, 2016

That's impossible with the wxGrid I think.

Collaborator

ericwa commented Sep 25, 2016

It's possible to make text bold in EntityAttributeGridTable::GetAttr by doing attr->SetFont(GetView()->GetFont().Bold());:

To clarify: what I was suggesting is, for any text that is currently rendered in black, additionally make it bold, so that it's unambiguous even when there is a selection in the table.

screen shot 2016-09-25 at 3 30 46 pm
Not sure if you like the look of that though.

Owner

kduske commented Sep 26, 2016

I had tried that, not too fond of the look tbh.

Owner

kduske commented Sep 26, 2016

I have changed the default attributes to be italic in addition to being light gray. This sets them apart when selected, too. What do you think?

Collaborator

ericwa commented Sep 26, 2016

Nice. I think it's good enough to not cause confusion
screen shot 2016-09-26 at 4 04 14 pm

Owner

kduske commented Sep 27, 2016

Alright, I'll merge it.

kduske closed this Sep 27, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment