Map types in info.json files to valid HTML types

[harshkhandeparkar]

Please describe the problem (or idea)

Input types in some module info.json files are not valid html input types. This makes typing number type values inconvenient and invalid values can be submitted. Types like integer and string should be replaced with number, range or string.

image-sequencer/src/modules/Rotate/info.json

Line 6 in 611016b

"type": "integer",

Possible Fixes

Add a new function in examples/lib which maps different types to valid HTML types

Thank you!

[gitmate]

GitMate.io thinks the contributor most likely able to help you is @ccpandhare.

Possibly related issues are #408 (Typo in info.json), #27 (add fisheyegl module), #113 (Add a Brightness module), #157 (Add a Contrast module), and #158 (Add a Saturation Module).

[gitmate]

GitMate.io thinks the contributor most likely able to help you is @ccpandhare.

Possibly related issues are #408 (Typo in info.json), #27 (add fisheyegl module), #113 (Add a Brightness module), #157 (Add a Contrast module), and #158 (Add a Saturation Module).

[vibhorgupta-gh]

@harshkhandeparkar what values would htmlType assume? Given that the type property would be float, integer etc

[harshkhandeparkar]

htmlType can be generalised. integer, float can both use number or range. percentage, color etc. can be string.

[harshkhandeparkar]

@tech4GT @jywarren is this issue good? Is this being fixed in some other issue or PR?

[Mridul97]

If no one is working on this issue, can I take up this issue?

[harshkhandeparkar]

Absolutely. That will be awesome!

[Mridul97]

@harshkhandeparkar we cannot keep htmltype to be Number for input type float because it will not allow us to enter values like 1.5. So what should we keep htmltype when input type is float?

[harshkhandeparkar]

@Mridul97 range will be good for float type inputs

[harshkhandeparkar]

@Mridul97 if you are taking up #628 then can I work on this?

[Mridul97]

@harshkhandeparkar Actually I am almost done with this one, opening a pr!

[harshkhandeparkar]

Hmm actually even I had worked on this. Should we collab?

…

On Tue 8 Jan, 2019, 6:32 PM Mridul97 ***@***.*** wrote: @harshkhandeparkar <https://github.com/HarshKhandeparkar> Actually I am almost done with this one, opening a pr! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#570 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AhKOnyFc98ETgH6T-J4ACLEGyMOLYgnmks5vBJb5gaJpZM4Zk6ch> .

[harshkhandeparkar]

Tell me your branch name. We will compare our changes. I will open a pr to your branch. Lets merge our changes.

[Mridul97]

Branch name is htmltypes, I just opened a pr #633.

[harshkhandeparkar]

Look at my branch htmlType-in-info-files I have done many more other changes.

[Mridul97]

There are multiple commits to see changes like this becomes a bit inefficient, you can see my changes here #633.

[harshkhandeparkar]

I have opened a pr to your branch. Please check it out. Also please git merge upstream/main

[jywarren]

Hi, I'd like to suggest a slightly different approach, i'm sorry I didn't earlier but the rate of contribution is making my inbox a bit wild, apologies!

What if we maintain a utility function which maps the type to an htmlType, so that we can be more specific than htmlType (for reasons you've outlined here!) while not further complicating the info.json files, and not making the process of adding a new module any more complex?

We could create /src/util/inputTypeConversion.js perhaps... with some internal mapping with an assoc. array, like:

types = {
  'string': 'input',
  'text': 'textarea',
  'range': ...
}

And the util module could provide a method like:

inputTypeConversion.typeToHtmlType('string');

What do you think of this?

[jywarren]

I appreciate all your work on this! I think it's a great idea. Thanks for considering my suggestion.

[harshkhandeparkar]

@jywarren The idea you suggested is cool but same type of input can have more than one htmlType like e.g. 'integer' can have both 'range' and 'number' as input. What can we do in that case?

Update: opened a new pr

[harshkhandeparkar]

Update: opened a new pr
@jywarren I'll open a new PR

[harshkhandeparkar]

New pr #674

[jywarren]

ah, actually wouldn't the lookup would go from both range and number to integer, and not the other way?

…

On Tue, Jan 15, 2019 at 1:33 AM Harsh Khandeparkar ***@***.***> wrote: New pr #674 <#674> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#570 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ2LZq01JomzE5H9f3YvGeYd4cPSCks5vDXYHgaJpZM4Zk6ch> .

[harshkhandeparkar]

@jywarren I think your rebase caused some problem in the pr and no changes were made. Any thoughts? I will probably open a new pr by force pushing from local. I still have the changes there. Thanks

[jywarren]

Oh sorry! Yes please do.

…

On Sat, Feb 9, 2019, 2:52 AM Harsh Khandeparkar ***@***.*** wrote: @jywarren <https://github.com/jywarren> I think your rebase caused some problem in the pr and no changes were made. Any thoughts? I will probably open a new pr by force pushing from local. I still have the changes there. Thanks — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#570 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ3wpKD8lw8sjzqJSGMLFOoGcYOyYks5vLn4IgaJpZM4Zk6ch> .

[harshkhandeparkar]

Ok will do in a few minutes