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

Parameter box improvements #5119

Merged
merged 23 commits into from
May 29, 2024
Merged

Conversation

yw4z
Copy link
Contributor

@yw4z yw4z commented Apr 22, 2024

Adds missing / empty side texts

• Added "Layers" side text for

Top shell layers
Bottom shell layers
Number of slow layers

• Added "Layer" side text for

Full fan speed at layer

• Added "mm" side text for

Mesh margin
Minimum wall length

• Added "°C" side text for

Softening temperature

Fixes combo boxes without arrows

Support > Style
Screenshot-20240423155322

Filament Supports
Screenshot-20240423155447

Support > Advanced
Screenshot-20240423155536
Screenshot-20240423155541

after fix shows dropdown arrows if there is no icon for dropdown item

Screenshot-20240427173425

Converts double input boxes to regular input boxes

Screenshot-20240424134934

Screenshot-20240424134941

Screenshot-20240424134956

After Fix
Screenshot-20240524184001
• Used icons for X / Y instead text. input boxes aligns perfectly with other components
• Didn't use combined side text because it not matches style with other components
• Solves styling problem and gives same result on all platforms

Preview for Printer Settings > Printable Area > Set

before after
Screenshot-20240427173214 Screenshot-20240427172935

Also matched style of Create printer window
• Used text align on left like on other components
• Matched width of input boxes with other components

before after
Screenshot-20240427173123 Screenshot-20240427173049

Questionable areas for discussion (EXCLUDED FROM COMMIT)

• Converting these to spin boxes. Currently it combines combo box and input box and it has weird usage and no need to use on this ones because dropdown only shows numbers without any description. Using spin boxes better because other layer related input boxes uses this too

Top interface layers
Bottom interface layers

Screenshot-20240423021048

Screenshot-20240423021146

I noticed there is a "Same as Top" option for Bottom interface layers. I guess i will revert back to default with your comment

not rendered well with side text
Screenshot-20240423181955

solution with spin box. there is an explanation required on tooltip for -1 equals same as top
Screenshot-20240423182156

Also this ones combines combo box and input box. like i said its weird but helpful on this ones compared to interface layers ones

sparse_infill_line_width
infill_anchor_max
infill_anchor

If you planning to add more pre defined values for value boxes in future maybe increasing width of input boxes will be a good choice because value + dropdown arrow + side text looks very dense

Screenshot-20240423014410

I tried to make use them as regular input box in my UI commit but probably revert back with your comment

• Added "Layers" side text for
	Bottom shell layers
	Number of slow layers
	Top shell layers
• Added "Layer" side text for
	Full fan speed at layer
• Added "x" side text for ratios. This one looks nice imo
	Internal bridge flow ratio
	Bridge flow ratio
	Top surface flow ratio
	Bottom surface flow ratio
	Flow ratio
	Scarf joint flow ratio
• Added "mm" side text for
	Mesh margin
	Minimum wall length
• Added "°C" side text for
	Softening temperature
• Converted these to spin boxes. Currently it combines combo box and input box and it has weird usage. Using spin boxes better because other layer related input boxes uses this too
	Top interface layers
	Bottom interface layers
@discip
Copy link
Contributor

discip commented Apr 23, 2024

  1. • Added "Layers" side text for

    • Added "Layer" side text for

    Regarding all the 'side texts', you should consider the context:

    If the corresponding unit is used in the description in front of the value, there is no need to put it additionally thereafter.

    image

  2. • Added "mm" side text for

    &

    • Added "°C" side text for

    👍

  3. • Converted these to spin boxes ...

    • Due to the mentioned reason in 1., no need for 'side texts' here as well.
    • Since a drop-down allows you to access all the values ​​more quickly without having to click through them individually, as would be the case using a spin box, please leave this as is.
  4. I tried to make use them as regular input box in my UI commit but probably revert back with your comment

    Please don't remove the arrow.

@yw4z
Copy link
Contributor Author

yw4z commented Apr 23, 2024

@discip then we should delete these with your logic

Screenshot-20240423135607

Screenshot-20240423141648

side text generally uses units and its unit "Layer" / "Layers" in my perspective

problem is combining combo box and and input box and parameter value boxes too narrow to fit multiple components

about spin boxes you can set value faster with them compared to combo box

@discip
Copy link
Contributor

discip commented Apr 23, 2024

  1. then we should delete these with your logic

    Absolutely! 😃👍
    I haven't checked every other possible case, but since you're at it, it would be great if you maintained the tendency you've demonstrated to make everything as consistent as possible. 😊

  2. side text generally uses units and its unit "Layer" / "Layers" in my perspective

    To demonstrate what I mean:
    Compared to e.g.: length, where you could have mm or cm, in which case it makes perfect sense to put the unit in the text box, even though the description contains the word length, the 'unit' layers doesn't need any further specification.

  3. problem is combining combo box and and input box and parameter value boxes too narrow to fit multiple components

    I'm having no issues using the current setup. 🤷🏻

  4. about spin boxes you can set value faster with them compared to combo box

    1. It depends on the amount of values the list contains.
    2. In most cases you would need 2 clicks to get any value with a drop down opposed to a spin box.
      e.g.: If your current value is 1 you would need to click 4 times to get to the 5 with a spin box. With a drop down however, you click the arrow and 5 (2 clicks)!

I'm really curious about what others think about these changes.

@yw4z
Copy link
Contributor Author

yw4z commented Apr 23, 2024

  1. about spin boxes you can set value faster with them compared to combo box

    1. It depends on the amount of values the list contains.
    2. In most cases you would need 2 clicks to get any value with a drop down opposed to a spin box.
      e.g.: If your current value is 1 you would need to click 4 times to get to the 5 with a spin box. With a drop down however, you click the arrow and 5 (2 clicks)!

you just count mouse clicks. you should also count mouse movements as steps

@yw4z yw4z marked this pull request as draft April 27, 2024 14:41
@yw4z yw4z marked this pull request as ready for review April 30, 2024 10:05
@yw4z
Copy link
Contributor Author

yw4z commented Apr 30, 2024

@SoftFever this commit also ready for review. also there is a "Questionable areas for discussion" section on first post

@yw4z
Copy link
Contributor Author

yw4z commented May 18, 2024

@SoftFever should i split this commit to separate commits
• Add missing side texts for parameter boxes
• Fix combo box arrows not appearing if there is no icon for drop down list item
• Convert double input boxes for points to regular input boxes
• Better color for disabled elements on dark mode (Will move this to new color fixes commit)
• Match text color of disabled combo box (Will move this to new color fixes commit)

@@ -816,6 +816,7 @@ void PrintConfigDef::init_fff_params()
def = this->add("bottom_shell_layers", coInt);
def->label = L("Bottom shell layers");
def->category = L("Strength");
def->sidetext = L("layers"); // ORCA add side text
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works really good 👍

@@ -918,6 +919,7 @@ void PrintConfigDef::init_fff_params()
def->category = L("Quality");
def->tooltip = L("Decrease this value slightly(for example 0.9) to reduce the amount of material for bridge, "
"to improve sag");
def->sidetext = L("x"); // ORCA add side text
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find add "x" side text strange.
Can we remove x side texts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is a % used for percentage. x fits well for ratio imo. also i tried to show values as "1.00" to indicate its float numbers but they always rounded to "1"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% and other units like mm are common and therefore look natural.
The x looks very strange and confusing as it displays like 1 x.
We shouldn't add it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

google suggest few alternatives for multiplier symbol.
Multiplication sign definition: the symbol ( ⋅ ), ( × ), or ( ∗ )
But ratio is completely different thing. it mostly suggest proportional ratio and its symbol is " : "

1     mm
1     x
1     ⋅
1     ×
1     ∗

ofc i would not die if we dont add it. still looks nice imo :)

here is degree, mm, % and x in same view

Screenshot-20240525033427

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think the x is confusing here :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good one!
One suggestion though, can we make the font easier to read? the current square font may cause confusion IMO

@yw4z
Copy link
Contributor Author

yw4z commented May 24, 2024

@SoftFever updated point control icons
before
Screenshot-20240524182305

after
Screenshot-20240524184001

@SoftFever
Copy link
Owner

@yw4z
What's the status of the PR?
Could we remove the x sidenote?

@yw4z
Copy link
Contributor Author

yw4z commented May 28, 2024

@SoftFever yes i will remove them but quite busy atm. probably complete changes on PRs on end of week

@yw4z
Copy link
Contributor Author

yw4z commented May 29, 2024

@SoftFever reverted changes for "x" side text and removed color related fixes because they need additional changes. i will send color related issues as another commit because i found 10+ more

btw why this one not uses percentages. it will make it easier to understand
Screenshot-20240529080349

Copy link
Owner

@SoftFever SoftFever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SoftFever SoftFever merged commit c14ae13 into SoftFever:main May 29, 2024
8 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants