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

small UX update for optionsdialog #1649

Merged
merged 1 commit into from Aug 14, 2012
Merged

small UX update for optionsdialog #1649

merged 1 commit into from Aug 14, 2012

Conversation

Diapolo
Copy link

@Diapolo Diapolo commented Aug 2, 2012

  • add enableApplyButton() and disableApplyButton() to optionsdialog.{h/cpp}
  • they are used to ensure the Ok button does not get disabled, when Apply needs to be disabled (standard UX should allow Ok always to dismiss the dialog and only disable it, when we have a faulty proxy IP)
  • disable Apply after initially loading the settings, as nothing new needs to be saved
  • remove orphan settings from optionsdialog.ui that are default anyway

- add enableApplyButton() and disableApplyButton() to optionsdialog.{h/cpp}
- they are used to ensure the Ok button does not get disabled, when Apply needs to be disabled (standard UX should allow Ok always to dismiss the dialog and only disable it, when we have a faulty proxy IP)
- disable Apply after initially loading the settings, as nothing new needs to be saved
- remove orphan settings from optionsdialog.ui that are default anyway
@@ -147,6 +150,16 @@ void OptionsDialog::setMapper()
mapper->addMapping(ui->displayAddresses, OptionsModel::DisplayAddresses);
}

void OptionsDialog::enableApplyButton()
{
Copy link
Member

Choose a reason for hiding this comment

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

Why define a function (two functions, even) for what is one (pretty straightforward) line anyway? Or do you plan to extend this?

Copy link
Author

Choose a reason for hiding this comment

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

It is used in connect(mapper, SIGNAL(viewModified()), this, SLOT(enableApplyButton()));, where I can't use ui->applyButton->setEnabled(true); directly, right?

Copy link
Member

Choose a reason for hiding this comment

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

You're right. It sucks that connect(mapper, SIGNAL(viewModified()), ui, SLOT(setEnabled(true))); is not possible.

Copy link
Author

Choose a reason for hiding this comment

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

So everything fine here :)?

@BitcoinPullTester
Copy link

The following is an automatic comment from the Bitcoin Pull Tester.
If you believe it is in error, please contact jenkins@bluematt.me

This pull passed automatic sanity-tests!
This means it merges cleanly onto current master, builds and unit-tests pass
You can find the test log and build output at http://jenkins.bluematt.me/pull-tester/4aaa4313e7edf5d23143e393efd2d5892d5dde48

@laanwj laanwj closed this Aug 14, 2012
laanwj added a commit that referenced this pull request Aug 14, 2012
small UX update for optionsdialog
@laanwj laanwj merged commit 0825aee into bitcoin:master Aug 14, 2012
suprnurd pushed a commit to chaincoin-legacy/chaincoin that referenced this pull request Dec 5, 2017
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants