-
Notifications
You must be signed in to change notification settings - Fork 31
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
Colorbar feature #605
base: master
Are you sure you want to change the base?
Colorbar feature #605
Conversation
… adding inverting of colorbar
… adding inverting of colorbar
src/js/views/lutpicker.js
Outdated
this.render(); | ||
}); | ||
this.loadLuts(); | ||
this.render(); |
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 change now calls this.render()
before the luts have loaded. Haven't tested to see if this actually causes a bug, but I think this will be fixed with:
this.loadLuts().then(() => this.render());
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.
That may indeed, I added your fix. Thanks
I wonder if you want to allow users to choose the channel for the colorbar (if you have more than one active)? It's even possible to imagine someone wanting 2 or more colorbars at once, for different channels, but I guess that's a bit edge case (you would just do a split-view figure). |
I assumed that colorbars only made sense with a single channel (especially if the channel uses a LUT). I guess your trick is a good enough workaround if someone has the use case, but I wouldn't make it the normal "use case" to show one colorbar per active channel. Instead of adding a parameter for the channel to show a colorbar for, I would add a doc/tooltip somewhere indicating that the first active channel is used for the colorbar. |
I had to try quite hard to "break" this, but I managed to paste some invalid text into the "Spacing" field, which gave me figure JSON Actually, I'm not sure that any of the number field placeholder attributes are really useful as strings since they're mostly not readable and a placeholder should really be a suggestion of a default value? (I see that I've done it the same way with the Scalebar fields too, but I think this is also incorrect. See https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/placeholder). The form layout needs tweaking a bit to get all the input fields to the same size and to align them to the left. I think you need As discussed last week, I think we are running out of space in the |
The LUT png is resized big enough to provide for bars of 50px thick, but if you go higher you get other ones showing. E.g. 100 wide: You could simply stretch the lut png a bit more, or limit the thickness. Probably both. NB: even with a thickness of 50 you get the next LUT creeping in as there's anti-aliasing at the join. |
A couple more thoughts... I tried batch editing 2 panels at once. In the previous screenshot, I selected the top 2 panels (blue and green) and wanted to make them consistent by choosing the same number of Ticks and Font size but not changing other properties such as position. Then I checked the behaviour for Scalebar and found that this doesn't work Scalebar - If you make a change to one property, it tries to apply ALL of them (same as Colorbar). In some ways, Scalebar is worse because for values that are not the same between selected panels E.g. Length - it shows NO value when those panels are selected together. The idea here is that the UI lets you know when these values are different between selected panels (which Colorbar doesn't do). BUT, when you edit a different property (E.g. Height) the So, it's probably best just to leave it as is. In the scenario above, I was actually happy to have all the properties consistent between the 2 colorbars, except for position. So I just had to re-select the first panel and reset it's colorbar position, which wasn't a big deal. |
This PR implements the optional display of a colorbar next to a panel. The colorbar shows the mapping between pixel values and LUT ("normal" color included). This is work from @MinaEnayat and me.
There are still adjustments to be made, but this is functional and mature enough that we thought it was ready to receive comments/ideas.
Implemented features:
A spine with the ticks and labels gives a visual clue when the LUT is the same color as the page background, but only on one side to avoid confusion with the LUT when the LUT is the same color as the spine:
![image](https://private-user-images.githubusercontent.com/10853316/393766639-ce890474-425a-4182-831f-9da9ba83185b.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5MTQ0MzcsIm5iZiI6MTczODkxNDEzNywicGF0aCI6Ii8xMDg1MzMxNi8zOTM3NjY2MzktY2U4OTA0NzQtNDI1YS00MTgyLTgzMWYtOWRhOWJhODMxODViLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMDclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjA3VDA3NDIxN1omWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTVkMjU5NWExMDJlODIyOWNmYjEyNDA0MDBkNTk1ZDZkY2MyYTQyMDk3ZWIyOTU3NDk3Mjc1MTNmZjMwZmQ5MmEmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.fsP-M1WyRu5nvwZPSAqmlZlBezZEKMibdu8onqx475U)
Suggestions for tests:
Known issues / work left:
-log10(end-start / num_ticks)