Add toggle color layer visibility button #3943
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -68,7 +68,21 @@ | |||
| "properties": { | |||
| "name": { "type": "string" }, | |||
| "category": { "enum": ["color", "segmentation"] }, | |||
| "elementClass": { "enum": ["uint8", "uint16", "uint24", "uint32", "uint64", "float,", "double", "int8", "int16", "int32", "int64"] } | |||
| "elementClass": { | |||
There was a problem hiding this comment.
my ide beautifies code again 🤔
As this is accepted by prettier I suggest to keep it this way.
There was a problem hiding this comment.
Hmm, in the long run, this can be quite cumbersome. This is especially prone to merge conflicts. Can you please disable any IDE auto-formatting which differs from prettiers behavior?
| @@ -39,8 +39,8 @@ import constants, { | |||
| } from "oxalis/constants"; | |||
| import messages, { settings } from "messages"; | |||
|
|
|||
| const Panel = Collapse.Panel; | |||
| const Option = Select.Option; | |||
| const { Panel } = Collapse; | |||
There was a problem hiding this comment.
also done by my ide
daniel-wer
reviewed
Mar 25, 2019
There was a problem hiding this comment.
Nice work :)
I have a few thoughts about this feature:
- Could you add a yellow/orange
warningicon with a tooltip next to the "Color Layers" header if any of the color layers is disabled? I'm afraid of users disabling layers by accident or forgetting about it. - Should we think about this more in terms of "Enabling/Disabling" the layer? I would, for example, disable all the layer controls of one layer (brightness, contrast etc.) of one layer if it is disabled. Also in the future we'll likely not load data for disabled layers as a performance optimization - the naming would make this clearer imo. Additionally, users would be less confused why there is an opacity slider and also a visibility toggle if the toggle would have the enable/disable connotation. What do you think? (Also @philippotto )
- Lastly, @georgwiese could you try this feature on the linked test branch? Does this work for you, any feedback? :)
frontend/javascripts/oxalis/view/settings/dataset_settings_view.js
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/settings/dataset_settings_view.js
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/settings/dataset_settings_view.js
Outdated
Show resolved
Hide resolved
|
Works for me! Cool! I'd still need that the layers are not blended additively but with alpha blending, but gues that's another issue. |
|
@daniel-wer I applied all your feedback. Could you please review my changes again? 🙂 |
daniel-wer
approved these changes
Mar 27, 2019
frontend/javascripts/oxalis/view/settings/dataset_settings_view.js
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/settings/dataset_settings_view.js
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.js
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/view/settings/setting_input_views.js
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a button to each color layer that enables/disables the layer's visibility. This is done by adding the property
isVisibleto color layer configurations. => changed toisDisabled.URL of deployed dev instance (used for testing):
Steps to test:
Issues:
[ ] Updated migration guide if applicable[ ] Adapted wk-connect if datastore API changes[ ] Needs datastore update after deployment