-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
frontend: Move transition preview button to button box #8041
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
base: master
Are you sure you want to change the base?
Conversation
|
Overall I think this is a good idea, my only concern UX-wise is that it's right next to the Defaults button, which a user might accidentally click instead (though Cancel would undo it, so it's probably safe). |
|
If people prefer this: |
|
Your proposed change sounds fairly reasonable exeldro. what do you think matt? |
|
I'd worry about the non-standard UX, it'd be a bit inconsistent with buttons on other dialogs. Overall though, I'm OK with either option. |
|
@Warchamp7 prefers the bottom-right approach |
ee8253b to
d889703
Compare
d889703 to
be58798
Compare
|
I've rebased this PR |
be58798 to
5cccc66
Compare
5cccc66 to
f09ddca
Compare
RytoEX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge fan of this approach hacking into the internals of QDialogButtonBox, especially since the layout of the standard buttons can vary between operating systems (and nowhere does Qt guarantee that the QBoxLayout this relies on even exists, that's just an implementation detail and could change at any point).
However QDialogButtonBox::addButton exists, which I think would be a much better solution.
Also, git commit authorship by exeldro should probably be retained, that appears to have been lost in a (manual?) rebase.
98c9055 to
023c363
Compare
023c363 to
1dec832
Compare

Description
Move transition preview button to button box


Before:
After:
Motivation and Context
It is more clear that the button does not belong to the transition properties.
The preview button stays visible while editing the transition.
How Has This Been Tested?
On windows 64 bit by opening the properties of a stinger transition
Types of changes
Checklist: