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

reordering and renaming certain menu items #7573

Merged
merged 14 commits into from
Dec 14, 2024

Conversation

discip
Copy link
Contributor

@discip discip commented Nov 25, 2024

Description

With this PR I tried to unify the behavior and appearance of the menu structure so that the entry that controls others is above them.
I hope that makes sense. 😊

Screenshots/Recordings/Graphs

before after
image image
image image
image image
image image

The colored markings are only to indicate the different placement. 😊

@Noisyfox
Copy link
Collaborator

Noisyfox commented Dec 1, 2024

This makes a lot more sence to me, cool!

@discip
Copy link
Contributor Author

discip commented Dec 2, 2024

@Noisyfox

This makes a lot more sence to me, cool!

Glad you see it that way too. 😊

@Ergonomicmike
Copy link

Ergonomicmike commented Dec 2, 2024

@discip Nice! I always like ergonomic improvements.

It's too late to add this idea now that all checks have passed. But perhaps if there's a next time? (I had posted this in early August, but it's gone now.) There is one more item to "unify."

The Compare presets feature is not in the same visual order as the Presets to compare. In the Presets, it's Printer at the top, Filament in the middle, and Process at the bottom. Whereas in the Compare presets window, it's Process at the top and Printer at the bottom. (See screen shot.) Confusing. I am guessing that two lines of code could be swapped to unify this?

Ergonomic improvement Orca compare feature

@discip
Copy link
Contributor Author

discip commented Dec 3, 2024

@Ergonomicmike
I finally found the part of code that needed to be changed to meet your request. 😊

This is how it looks now:

image

@SoftFever
Since this is about unifying the order of entries, I hope it's OK to include that here as well.
Please let me know if this is ok.

@Ergonomicmike
Copy link

@discip Cool! Thanks!

I can see where the printer, filament, material entries were swapped and I could have hacked that part myself. (But I don't know how to do a PR.) But I would totally have missed changing the order of things at line 1845. Glad to have experts here.

@Hammerfest
Copy link

I know logic isn't everything, but it always broke my brain the out of order option hierarchy

As someone who prefers the tree style visual layout of old, at least this brings some order to the chaos and its much appreciated, always jarring to see options above others that if not set mean they don't actually do anything (hence the tree layout comment)

Experience: I actually fumbled over skirt loops and a few others when I first started a short time ago, wondering why they didn't work

@discip
Copy link
Contributor Author

discip commented Dec 5, 2024

@Ergonomicmike

Glad to have experts here.

🤣

By no means I'm an expert! I just searched for the specific term and tried changing what ever seemed to fit until it worked. 😊

But I would totally have missed changing the order of things at line 1845.

This is the line that actually made the change.
And in the interest of uniformity, I left the changes in the other lines as they do not seem to be disruptive.

Copy link
Collaborator

@Noisyfox Noisyfox left a comment

Choose a reason for hiding this comment

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

LGTM

@Noisyfox Noisyfox merged commit ef28de6 into SoftFever:main Dec 14, 2024
16 checks passed
@discip discip deleted the reordering-certain-menu-items branch December 14, 2024 09:48
GiacomoGuaresi pushed a commit to gingeradditive/OrcaSlicer that referenced this pull request Dec 19, 2024
* reordered Z-Hop settings

* Update Tab.cpp

* Update PrintConfig.cpp

* reordering **`Compare presets`** to match the layout in the sidebar
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.

4 participants