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

Disable brush in merge mode #4770

Merged
merged 12 commits into from
Aug 19, 2020
Merged

Conversation

grittaweisheit
Copy link
Contributor

@grittaweisheit grittaweisheit commented Aug 17, 2020

This PR disables the brush when merger mode is enabled in a hybrid tracing.

URL of deployed dev instance (used for testing):

Steps to test:

  • open a hybrid tracing
  • select the brush tool
  • enable merger mode
  • the tool should change to "move" and the brush button should be disabled
  • disable merger mode
  • the brush should be available again

Issues:


Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Nice work 👍 I think a tooltip explaining why the buttons are disabled (in case they are) would be helpful to users. Otherwise they are left wondering why they cannot volume annotate. Message could be "Volume annotation is disabled while the merger mode is active." or something to that effect.

@@ -42,7 +49,9 @@ class VolumeActionsView extends PureComponent<Props> {
>
<RadioButton value={VolumeToolEnum.MOVE}>Move</RadioButton>
<RadioButton value={VolumeToolEnum.TRACE}>Trace</RadioButton>
Copy link
Member

Choose a reason for hiding this comment

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

The TRACE tool can also be used to volume annotate. It should be disabled in the same way.

@daniel-wer
Copy link
Member

I noticed that the button disabled style was broken for the dark theme header (not your fault!), because we overwrote some color values there and fixed that quickly, hope that is alright with you. Now disabled buttons are grayed out as usual and can be recognized without hovering :)

@grittaweisheit
Copy link
Contributor Author

Thanks a lot for fixing the button style!

I added the tooltip and disabled the trace tool as well :)

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Very nice, one small suggestion and then LGTM 👍

CHANGELOG.unreleased.md Outdated Show resolved Hide resolved
@grittaweisheit grittaweisheit merged commit d535df7 into master Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disable brush in hybrid tracings when merge mode is activated
2 participants