-
Notifications
You must be signed in to change notification settings - Fork 7
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
UI layout update #165
UI layout update #165
Conversation
…nimated/agave into feature/formlayouts
@@ -191,22 +193,25 @@ private slots: | |||
QSlider m_slider; | |||
}; | |||
|
|||
class Controls | |||
class AgaveFormLayout : public QGridLayout |
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.
introducing this class allowed finer control over laying out collections of input controls and lets us ensure some consistency across the ui.
@@ -287,14 +284,19 @@ QNumericSlider::QNumericSlider(QWidget* pParent /*= NULL*/) | |||
|
|||
m_slider.setOrientation(Qt::Horizontal); | |||
m_slider.setFocusPolicy(Qt::StrongFocus); | |||
m_spinner.setDecimals(4); | |||
m_spinner.setDecimals(2); |
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.
these inputs will now get 2 decimals by default and they have to explicitly change that where it makes sense to.
// deal with a macos style bug which causes this button to misalign in some situations | ||
setAttribute(Qt::WA_LayoutUsesWidgetRect); |
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.
Is this still a bug with the fusion style?
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.
It's not a problem with fusion, but it still doesn't hurt to leave it in. If we ever switch back it would suddenly break something.
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.
LGTM, thank you for the occasional comments explaining your changes!
QObject::connect( | ||
m_lt0gui.m_sizeSlider, &QNumericSlider::valueChanged, this, &QAppearanceSettingsWidget::OnSetAreaLightSize); | ||
|
||
m_lt0gui.m_distSlider = new QNumericSlider(); | ||
m_lt0gui.m_distSlider->setStatusTip(tr("Set distance for area light")); | ||
m_lt0gui.m_distSlider->setToolTip(tr("Set distance for area light")); | ||
m_lt0gui.m_distSlider->setRange(0.1, 100.0); | ||
m_lt0gui.m_distSlider->setRange(0.1, 10.0); |
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.
Out of curiosity, why did you change this value from 100 to 10?
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.
This value is a distance from light source to volume. The volume size is normalized to 1.0 so putting the light 100 units away is not very useful. It keeps the slider range more manageable for practical numbers. The good news is that modifying the slider range does not hurt the ability to load and save values outside this range.
another pre-release 1.6 polish update.
Estimated time to review: 15min or less