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

Maintenance issues from SliceViewer changes #12324

Closed
1 task done
OwenArnold opened this issue Apr 7, 2015 · 1 comment · Fixed by #12977
Closed
1 task done

Maintenance issues from SliceViewer changes #12324

OwenArnold opened this issue Apr 7, 2015 · 1 comment · Fixed by #12977
Assignees
Labels
Framework Issues and pull requests related to components in the Framework Maintenance Unassigned issues to be addressed in the next maintenance period.
Milestone

Comments

@OwenArnold
Copy link
Contributor

This issue was originally TRAC 11485

This ticket is blocked by :

  1. SliceViewer.cpp is already too long. The code that applies icon switching should be refactored out into functions. Reading the function calls should make the behaviour a lot more readable than it currently is.

This is what it currently looks like in one place:

if (m_peaksPresenter->size()>0)  {
    icon.addFile(QString::fromStdString(g_iconPeakListOn),
                   QSize(), QIcon::Normal, QIcon::On);
    ui.btnPeakOverlay->setIcon(icon);
    ui.btnPeakOverlay->setChecked(true);
  }  else {

    icon.addFile(QString::fromStdString(g_iconPeakList),
                   QSize(), QIcon::Normal, QIcon::Off);
    ui.btnPeakOverlay->setIcon(icon);
    ui.btnPeakOverlay->setChecked(false);
  }

What it should look like is something of the form:

if (m_peaksPresenter->size()>0)  {
    selectPeakOverlayButton();
  }  else {
    unselectPeakOverlayButton();
  }
  1. You have introduced a hard-coded magic number into SelectWorkspacesDialog.h. In the SliceViewer your are NOT using a QDialog in a polymorphic way. Therefore you can introduce new methods onto SelectWorkspacesDialog to indicate that a custom button is being used.
@OwenArnold
Copy link
Contributor Author

@NickDraper (2015-04-27T08:10:35):
Moved to R3.5 at the R3.4 code freeze

@OwenArnold OwenArnold added Framework Issues and pull requests related to components in the Framework Maintenance Unassigned issues to be addressed in the next maintenance period. labels Jun 3, 2015
@OwenArnold OwenArnold added this to the Release 3.5 milestone Jun 3, 2015
NickDraper added a commit that referenced this issue Jun 30, 2015
This answers point 1 from the issue

Point 2 I have discussed with Martyn for a second opinion.
The way it is currently implemented fits best with how Qt dialogs work
with return values, as they just return done(int retVal).  Qt has set
return values for Ok and Cancel, Yes and No, but no option for a custom
response.
The static constant is well away from the existing values and allows this
extension cleanly.

re #12324
NickDraper added a commit that referenced this issue Jul 1, 2015
QWidget->setIcon does not actually set the icon!

re #12324
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues and pull requests related to components in the Framework Maintenance Unassigned issues to be addressed in the next maintenance period.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants