FIX: Histogram module selected value not displayed

[VladimirMikulic]

Fixes #1295

The histogram module values are "true" and "false".
jQuery would coerce them to Boolean true and false instead of String
"true" and "false" which would result in empty value being displayed.

@keshav234156 could you review this? Thanks.

[codecov]

Codecov Report

Merging #1510 into main will not change coverage.
The diff coverage is 41.17%.

@@           Coverage Diff           @@
##             main    #1510   +/-   ##
=======================================
  Coverage   66.62%   66.62%           
=======================================
  Files         129      129           
  Lines        2661     2661           
  Branches      428      428           
=======================================
  Hits         1773     1773           
  Misses        888      888

Impacted Files	Coverage Δ
src/modules/Blend/Module.js	61.01% <41.17%> (ø)	⬆️

[keshav234156]

@VladimirMikulic @harshkhandeparkar cause of the problem seems to be correct. But was just wondering, we also have a "select" type option in Edge-detect.In Edge-detect datatype is identified as a string but in Histogram, data-type is identified as Bool. Do you know why that happening?

[harshkhandeparkar]

I'll have to check. I'll check later.

…

On Wed, 15 Jan, 2020, 10:56 PM keshav234156, ***@***.***> wrote: @VladimirMikulic <https://github.com/VladimirMikulic> @harshkhandeparkar <https://github.com/HarshKhandeparkar> cause of the problem seems to be correct. But was just wondering, we also have a "select" type option in Edge-detect.In Edge-detect datatype is identified as a string but in Histogram, data-type is identified as Bool. Do you know why that happening? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1510?email_source=notifications&email_token=AIJI5H6TLS3U2W5CAXPMMT3Q55BLLA5CNFSM4KHGIRV2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJBDYKQ#issuecomment-574766122>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIJI5HYFUJV7M3QPAMCP23TQ55BLLANCNFSM4KHGIRVQ> .

[rishabhshuklax]

image-sequencer/src/modules/Histogram/Module.js

Line 14 in bfff7e7

options.gradient = JSON.parse(options.gradient);

Error is because of this line

[VladimirMikulic]

Thanks @blurry-x-face. Yes, that is the error and I've updated the PR accordingly but I'll still keep my String constructor in defaultHtmlStepUi. It will serve like a security mechanism, if a programmer forgets to parse the value as a string, it will do it for him since everything in the DOM is a string.

@keshav234156 this is ready, could you also approve this on GCI dashboard? Thanks.

[keshav234156]

LGTM!!

[keshav234156]

@VladimirMikulic Let's protect this with a UI test. Would you like to add a UI test for the test which check that all the modules the default options are displayed . I can publish a hard task related to this on the GCI dashboard if you want.

[VladimirMikulic]

@keshav234156 I can't promise anything, but if you publish it I'll see what I can do.

[rishabhshuklax]

@keshav234156 I'll be happy to write those UI tests!

[keshav234156]

@blurry-x-face Ok, Go Ahead!!

[keshav234156]

@jywarren it's ready for your final review!!!

[jywarren]

This is weird. I used to see this:

And i still see that on #1453

Here I see this:

And there's no option to Update branch -- do you know why? Can you update it yourself by rebasing?

Thanks!

[VladimirMikulic]

@jywarren now it should be fine. Thanks.

[jywarren]

I'm going to try to squash and merge. Maybe it just can tell it doesn't need to re-run checks?

[VladimirMikulic]

👍