add feature to select units

arunavabasucom / radis-app

A web app for high-resolution infrared molecular spectra using RADIS

https://radis.app

GNU Lesser General Public License v3.0

11 stars 16 forks source link

add feature to select units #645

Closed arunavabasucom closed 2 years ago

arunavabasucom commented 2 years ago

Fixes #147

https://user-images.githubusercontent.com/73842340/187072868-c0d9cbbe-62bb-4f71-af43-eefb3a119517.mp4

erwanp commented 2 years ago

Hello, could you share some screenshots ?

arunavabasucom commented 2 years ago

@suzil we can merge this !! 😀

erwanp commented 2 years ago

Nice !

Design : can you remove the underline under the units button ?
Test : can you also show a screenshot with slit unit being updated when you change nm/cm-1 in the main waverange ?

erwanp commented 2 years ago

About wavenumbers :

if using nm or cm-1, is the plot correctly plotted too ?

Warning :

If changing waverange from cm-1 to nm -1; or from nm to cm-1 ; we'll have overlay problem (now that we have #452 ). Solutions :

either not be able to plot on the same plot anymore. . Changing wavenumber unit should grey to the Add To Plot option.
or keep it, but Radis make sure to convert the units. So if 1st spectrum was computed in cm-1 and plotted in cm-1 ; and 2nd spectrum was computed in nm, it should also be plotted in cm-1. This sounds hard to implement, so just go for (1) .

arunavabasucom commented 2 years ago

@erwanp i done that thing !!

erwanp commented 2 years ago

Please show a video to confirm.

If waverange changes from cm-1 to nm --> Add to plot becomes non visible
After pressing "New Plot" in nm; and the spectrum is computed, then "Add to plot" becomes visible again
If waverange changes from nm to cm-1 --> Add to plot becomes non visible
After pressing "New Plot" in cm-1; and the spectrum is computed, then "Add to plot" becomes visible again

Also, show this :

https://github.com/suzil/radis-app/pull/645#issuecomment-1229451220

suzil commented 2 years ago

For the unit label on the plot, we don't want to show the prefix from the library but instead just want to show the unit. Maybe we could transform the string to just include the last bit after the period? The equivalent of this but in JavaScript: s.split(".")[-1]

erwanp commented 2 years ago

Can cm-1 appear at cm⁻¹ ?

suzil commented 2 years ago

@erwanp Should we change the min/max values for the slider for wavelength? Right now we have a min of 500 and a max of 10,000. Does that capture what the user would input for wavelength?

erwanp commented 2 years ago

Good point ! actually 500 cm-1 is 20,000 nm and 10,000 cm-1 is 1,000 nm. The min/max should be changed accordingly.