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

Openscad Customizer Improvements #1781

Open
5 of 10 tasks
amarjeetkapoor1 opened this issue Aug 24, 2016 · 31 comments
Open
5 of 10 tasks

Openscad Customizer Improvements #1781

amarjeetkapoor1 opened this issue Aug 24, 2016 · 31 comments

Comments

@amarjeetkapoor1
Copy link
Member

amarjeetkapoor1 commented Aug 24, 2016

These are suggested customer improvements, after the main customizer branch has been merged with master.

Topics that could be handled after initial merge:

  • Fix initial display position / initial display status (should be hidden by default)
  • Consider removing underscores when displaying variable names (following Thingiverse behavior) (see forum post)
  • Order of both Parameters and Groups, most suggestions are to have same order as in code, alphabetical might be usable in some cases. Maybe we can have some toolbar to toggle the order? (see forum post)
  • Show customizer automatically after loading a script with parameters?
  • Default values settings could be just another Set of Parameter, an always present one.
  • Removing a preset causes preview rendering to occur twice in a row
  • Consider rewriting all use of Expressions in the customizer to simply use Value objects. That will make it simpler to deal with literals, as all Value objects are literals.
  • Look into reducing use of multiple inheritance (e.g. class ParameterWidget : public QWidget, public Ui::ParameterWidget, public ParameterExtractor, public ParameterSet)
  • Add a decimal input widget (a 'free input' number widget with a decimal validator, or are there better solutions?)
  • When creating a customizer for the first time (without saving the .scad file), the "+" and "-" buttons are unresponsive. Saving the file first fixes this, but this should be more intuitive.

Feature requests that may/will only be possible with native syntax:

  • conditionally show/hide parameters based on other parameter selections

GSoC Pull request see: #1751


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@PRouzeau
Copy link
Contributor

Here some suggestions for improvement:
1/ The widgets shall be presented in the order defined by the programmer, not in alphabetical order, to let the programmer decide the logic.

2/ The variable name display shall be in option, as with a label, the user does not need to know the variable name

3/ There shall be an option to have the label on the left of the widget, as is traditional in input screens, I propose the following syntax.
// label on top (existing)
//- Label on left
//+ label on right
//-R label on left, with widget aligned on right
The latter will be useful for tick box or radio buttons as it allow more text. For other widgets, the width shall be adapted to the content.

4/ The border around each widget use a lot of space and it shall be removed (or set as an option). The label shall be closer to the widget and proper vertical space management will make the discrimination with less wasted space. This is even more important if the label is set on the left of the widget.

5/ It can be useful to group some variables within a border/frame if they belong to a ‘functional group’ . Such group may have an optional title. This might be the same role as tabs, but within the same screen view, so a lower order structure. Maybe use /* [[group]] */

6/ A variable named ‘part’ is often used to select printable part. It may got a special treatment, as being the first user selection. Notably it shall not be inside tabs, but above them (in the header ?). Maybe when there are tabs, all variables defined before first tab definition shall be above tab selectors ?

7/ General ergonomy : There is no real point to have the input font size much lower than the label. They are information of equal importance for usability, so they may be in same font size, or with very low difference.

8/ General ergonomy: The arrows of the spin widget are extremely small, a tiny target difficult to reach for people not very handy or people using trackballs. Even if it looks less elegant, two side by side square buttons of the height of the widget shall be used. If you use same font for input as for label, that will make reasonable target size.

9/ Above comments may lead to more user options, so a preference window may be created. These preferences shall be attached to the .scad FILE, not to the OpenScad program, so another option is to create a syntax to define customizer preferences inline.

openscad_interface01b openscad_interface02b openscad_interface03b

@PRouzeau
Copy link
Contributor

I think we also need 'free input' number widget as you sometimes need to enter decimal values, which are not practical with spinbox.

@kintel
Copy link
Member

kintel commented Aug 25, 2016

@PRouzeau A small note: The purpose of this issue is to discuss the fixes necessary before merging to master. To make this more clear, we should move any non-critical, cosmetic or future wishes into separate issues for each topic.

@amarjeetkapoor1
Copy link
Member Author

amarjeetkapoor1 commented Aug 25, 2016

@kintel I told him that he can add all the issues and features here in form of list. So, that everything would be together. And also shared the link of this issue on all major posts that If they found any issue they can report. My mistake.

@PRouzeau
Copy link
Contributor

Hello Marius,

I followed the recommendations done here :
https://amarjeetkapoor1.wordpress.com/2016/08/21/20th-june-2015-2/#comment-62

so you suggest that I open a new issue ? Or what shall be the channel
for feature requests ?

Regards, Pierre

Le 25/08/2016 à 16:35, Marius Kintel a écrit :

@PRouzeau https://github.com/PRouzeau A small note: The purpose of
this issue is to discuss the fixes necessary before merging to master.
To make this more clear, we should move any non-critical, cosmetic or
future wishes into separate issues for each topic.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1781 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ALLTlYcwCDuRSrelSZ0D92PYZTO3BM7Zks5qjahOgaJpZM4JruAG.


Pierre Rouzeau - Proud indigenous of old Europe

www.rouzeau.net http://www.rouzeau.net


L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

@t-paul
Copy link
Member

t-paul commented Aug 25, 2016

What about collecting all topics here for now and just categorize for needed for merge / to do later in the description on top? Once the needed for merge stuff is done, we can just move the remaining topics to a new issue and close this one.

@amarjeetkapoor1
Copy link
Member Author

@t-paul , @kintel we also need to categories the points in original issue itself.

@kintel
Copy link
Member

kintel commented Aug 25, 2016

I agree that we should split this off into issues internally, at least until we have officially shipped this.

@donbright
Copy link
Sponsor Member

I am trying to test this by building the "gsoc2016-refactored" branch but my "Edit/Preferences" menu doesn't show any "Features" tab.

@amarjeetkapoor1
Copy link
Member Author

@donbright It should show the Features tab if you are building it with.
qmake CONFIG+=experimental

@donbright
Copy link
Sponsor Member

Thanks very much.
I tried loading Candle stick and i am not sure if i did it right, is it supposed to look like this?

When I 'unhide' the customizer it shows this:

delme

When i 'x' the customizer it shows this:

delme2

@amarjeetkapoor1
Copy link
Member Author

No, It shouldn't be like this. It should be more like second screen shot that you shared.

but Same thing also happens with me but I used to think it is was due to some previewing problem in OpenSCAD as when I used to render it would render correctly but didn't preview correctly on making the customizer visible .

And this problem is not only with customizer only but also with other things like console, toolbar, editor.

@donbright
Copy link
Sponsor Member

There is another issue that is a bit of a problem. I ran the customizer on the candle and played around with the settings. I get this error when i try to switch branches.

don@serebryanya:~/src/openscad/binmaster$ git branch
  fbsdbuild
* gsoc2016-refactored
  master
don@serebryanya:~/src/openscad/binmaster$ git checkout master
error: Your local changes to the following files would be overwritten by checkout:
    examples/Parametric/candleStand.json
Please, commit your changes or stash them before you can switch branches.
Aborting

It looks like the act of testing the code results in changes to a textfile that is part of the git repository.

@amarjeetkapoor1
Copy link
Member Author

I think its not an issue as you would have got same if you had made changes to any .scad files given in examples. As when you played with Customiser you might have updated, added or deleted some set of parameters which are saved in stated JSON file. So, content of JSON file might have got changed.

@donbright
Copy link
Sponsor Member

donbright commented Sep 4, 2016

how is this going to work on a system where the user does not have administrative access and the example files are read only?

it is also going to be hard for someone building a test suite to deal with source code that is altered by the running of the test.

@kintel
Copy link
Member

kintel commented Sep 5, 2016

@donbright Good point. We should avoid writing files to the examples folder. This folder should be considered (and probably will be) read-only on most OpenSCAD installations.

@t-paul
Copy link
Member

t-paul commented Sep 5, 2016

How is that different from the *.scad files? If the file is read-only, it will not be possible to change/overwrite it. We discussed that scenario and it should be implemented already (allow json file to open read-only), but I did not yet test that specifically yet. Maybe we need some additional indication in the parameter GUI, but otherwise I don't see how this is special.

@amarjeetkapoor1
Copy link
Member Author

If the JSON file is in read only mode then it can't be read at present. So, it couldn't be edited when installed. And I have added a feature to the list in starting of this issue that we would allow read only JSON files to be read but not modified.

As in case of @donbright He modified the examples which were part of source code but not installed version.

@t-paul
Copy link
Member

t-paul commented Sep 5, 2016

Whops, yes, you are right, we listed it as open issue in #1781.

@kintel kintel mentioned this issue Sep 22, 2016
24 tasks
@TLC123
Copy link

TLC123 commented Oct 5, 2016

Feature requests: conditionally show/hide parameters based on other parameter selections .

@t-paul
Copy link
Member

t-paul commented Oct 5, 2016

@TLC123 added to the list, but this is not possible with the thingiverse syntax, so it's out of scope for the current implementation.

@donbright
Copy link
Sponsor Member

donbright commented Oct 6, 2016

@MichaelAtOz
Copy link
Member

This'll be the first time I looked at this.
Is there any doco somewhere?

@t-paul
Copy link
Member

t-paul commented Oct 6, 2016

@amarjeetkapoor1
Copy link
Member Author

amarjeetkapoor1 commented Oct 8, 2016

@kintel , Any suggestion related to two issues left for initial merge. For one related to ast file, I liked the suggestion by @t-paul that we can comment the native syntax code thats dumped in ast file.

 //@Parameter()
a=1;

Or any better suggestion that you and teepee have?

@kintel
Copy link
Member

kintel commented Oct 8, 2016

@amarjeetkapoor1 Ideally it should dump in the same format as it was read.

@amarjeetkapoor1
Copy link
Member Author

Yes, but it would be difficult task to dump comments correctly.

@t-paul
Copy link
Member

t-paul commented Oct 8, 2016

Trying to regenerate the Thingiverse comment format is probably quite ugly and may get into the way once we start with a native format. I see how it makes sense to ensure the generated AST dump is be compilable, but as it's mainly used for debug/tests is it worth the effort?

If the actual annotation format looks strange, we could drop that and just write comments with the data without trying to regenerate the exact format (which is especially difficult for the group markers I suspect).

@kintel
Copy link
Member

kintel commented Oct 8, 2016

The primary purpose of dumping AST files is for black box regression testing, as we could verify that "read-dump-read" gives the exactly same result.
If it's not feasible to output comments on the same format we read it, I suggest that we simply ignore any annotations when dumping AST files, and leave this as a future enhancement.

@jon-bondy
Copy link

I just checked it out, and agree wholeheartedly with the comments that the Customizer presentation should be much more compact (about 1/2 the height) and that the user should have the option to present the parameters in the order in the file. Wonderful new feature!

@kintel kintel moved this from In Progress to Required in OpenSCAD Next Release Sep 3, 2017
@kintel kintel moved this from Required to TODO - Required in OpenSCAD Next Release Sep 3, 2017
@kintel kintel moved this from TODO - Required to Evaluating in OpenSCAD Next Release Feb 28, 2018
@MichaelPFrey
Copy link
Member

I am going to update the todo list. For documentation:

@t-paul t-paul moved this from Evaluating to TODO - Required in OpenSCAD Next Release Nov 10, 2018
@kintel kintel removed this from TODO - Required in OpenSCAD Next Release Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants