[Merged] Sketcher Elements Widget rework

[paddle]

Overall looks great. I'm sure users will generally appreciate.
From the video, your 'point' filter seems to not work.
I indeed think the 'not applicable' icon shall be lighten.

If you can submit a PR for this change, I can review it.
Note to myself : time to improve caching in MultIcon.

Here are the pull request for both widgets :
Constraint : https://github.com/FreeCAD/FreeCAD/pull/7566
Element : https://github.com/FreeCAD/FreeCAD/pull/7567

Please note that I haven't tested on master as my master is broken. It did build at least... I had to solve a few conflicts due to wmayer changes to all sketcher files for modernisation. I solved them but as my build is broken it's not impossible that it will be broken completely. If it is consider testing on my branch : https://github.com/PaddleStroke/FreeCAD ... et_testing

Cheers

[paddle]

would it still be relevant to have this label with this changes? If not it should be removed, if yes we might need to revise the keyboard shortcut and the label. @abdullah can you perhaps clarify what this is supposed to do?

I don't feel strongly either regarding showing useless icons or not. But codewise it would be a lot of work and I'm not very motivated to change it.
For the icons I have no strong opinion. They can be modified easily down the line as it's only modifying the svg.

I reimplemented the Z feature, so that if you press Z while a line is selected, then selection will switch between edge, start and end. My understanding of the feature was from the code, so I'm not 100% sure what it did. But now I'm sure what it's supposed to do Though I haven't tested as Z still conflict and haven't tried to change the shortcut (which I could have done in insight).

[abdullah]

"Z switch to next valid type" : I'm actually not sure what it does. It seems it's broken because shortcut Z is used by something else. I think it changed from edge to next available type : start, end, mid depending on selected geo.

would it still be relevant to have this label with this changes? If not it should be removed, if yes we might need to revise the keyboard shortcut and the label. @abdullah can you perhaps clarify what this is supposed to do?

If you put your mouse inside the widget, the 'z' still iterates over the types (start, end, mid) in version:

Code: Select all

OS: Ubuntu 20.04.5 LTS (MATE/mate)
Word size of FreeCAD: 64-bit
Version: 0.21.
Build type: Release
Branch: unknown
Hash: 6d33a969bf5de9e958be14df59064147854f149e
Python 3.8.10, Qt 5.12.8, Coin 4.0.0, Vtk 7.1.1, OCC 7.5.2

The idea was to enable to select any edge and/or point combination only with the keyboard.

As for the "Auto switch to edge", the sense was to have a known starting position (edge), so that the number of 'z' presses would be deterministic. There was quite an amount of discussion ongoing during development (around 2014, I think).

I have not tried the feature yet, but it looks nice. I like the consistency between the constraint and elements widgets. I like that it is much more intuitive. I have not tried to use it with the keyboard yet. Ideally, it should be possible to "easily" perform selections only with the keyboard. I need to test it (and I welcome testing from others, so that we can identify potential problems as early as possible).

[adrianinsaval]

Ok that's a very nice functionality but it might not be working right now because I think Z is assigned to something else. The auto-switch to edge switch seems like it won't be necessary, IMO it should always select the edge when you click on the item in the list, you can use the buttons to go directly to one of the points or use the keyboard shortcut.

[abdullah]

I think it overall works well.

I would like to see one improvement to how the filter does not currently show the status. I think the collapsible filter is ok. Chrisb is going to be happy, I think, because vertical taskbar space is much better taken care of. However, I really miss an indication of the current filter selection when collapsed.

I was almost to report a bug, when I realised I had played with the filters and part was not showing. The only indication I see is the "checked" checkbox and depending on the content, maybe the order (the lack of some numbers that are filtered out).

Peek 16-10-2022 07-03.gif (238.15 KiB) Viewed 789 times

I wonder if anyone could suggest a good way of providing an overview of the filter selection (this is the "filter status") between the "Filters" checkbox and the "settings" icon. I think that would be a very nice improvement.

There is one small functionality that is lost and which may be possible to be implemented. With the old version, when selection was locked to one subelement type, one could hover over the element in the list and the subelement would be preselected. This, I think, was nice to identify what exactly we would select, if we add it to the selection. This is gone. But, I think it could be implemented by catching the hovering over the icons, so that when the mouse hovers an edge, or an endpoint, it preselects the relevant element in the 3D view. What do you think?

About the operation of the filter, I think we should add "Internal Alignment", because since the last release, internal alignment geometry is a different type, with different representation (it used to be just construction geometry). It should also help to keep the number of elements in the list shorter. Specially when working with B-Splines.

About the 'Z' shortcut, I think we can drop it. The rationale is that before 'Z' was necessary to iterate type. One had to iterate and the click on the item to select the subelement of that geometry. Now one just clicks. I see no advantage in having it and it would reduce the vertical size even further.

On the aesthetics. I have read the discussion above about only showing the subelements available to the element. I favour a matrix kind of representation like the one above, as subelements are always in a given position. I wonder if we could drop the X altogether, but leaving the space empty (so that it would still look like a matrix, but maybe it would appear less loaded). I am not the UI guy, just trying to be helpful in bringing different opinions together.

I welcome feedback (of the items above or others).

I will start the code review now in GH.

[abdullah]

I think I discovered one little bug. When clicking an "X", the edge is selected. I think this should not be case. I think it should do nothing instead. Does it make sense?

[onekk]

Hello, probably to indicate the filter applied a QLabel near the checkbox will suffice to show what filter are applied.

or some small icons that maybe could even substitute the dropdown list.

But for icons it probably more difficult to have consistent behaviour and appearance between different monitor redilution and "real estates" fonts are more easy to scale and a Qlabel has less vertical size used that a combobox, in my little knowledge of Qt.

I like this new organisation, it seem more clear and mire informations are available with one sight.

Regards

Carlo D.

[paddle]

Or instead of adding another label, we can just modify the checkbox text.
Filter: construction
Filter: construction + arc.
Ideally the text should not force the minimum size. ie just show text that can fit in current size.

[abdullah]

Yes it is about reusing the space, not increasing the space (width of the widget).

Icons can concentrate quite some information in a small space. What about an set of icons next to the label to show the state?

Also,maybe it does not need to be the whole detailed information. For example, one icon could show that all types of element (construction, normal, external, internal alignment) are selected, or that only some of them (so two icons). A second icon could show that geometry types (lines, circles, ...) are selected, or only some of them.

It is just an idea that came by. What do you think?

[onekk]

Or instead of adding another label, we can just modify the checkbox text.
Filter: construction
Filter: construction + arc.
Ideally the text should not force the minimum size. ie just show text that can fit in current size.

Or using as done in other softwares, some identifiers:

C >> Construction
A >> Arc
W >> whatever you want (Sorry I don't have time now to search for real names)

so CA >> construction + arc and CAW is construction + arc + wathever and so on. tooltip maybe could be a more extended explanation or a little "table" (made using monospace fonts) to illustrate the meanings.

Regards

Carlo D.