fix: Improve form inputs #129

mahatoankitkumar · 2024-06-19T07:26:33Z

Problem

Better form fields

enum → Toggle
boolean → dropdown with true and false options

Datron

I am thinking the input field should be made a component called DynamicInput that can change based on the JSON schema of an item. If that can be done in a follow up PR, I can go ahead and approve.

Datron · 2024-06-20T06:38:29Z

crates/frontend/src/components/dimension_form.rs

@@ -165,7 +165,7 @@ where
                            <textarea
                                type="text"
                                placeholder="JSON schema"
-                                class="input input-bordered mt-5 rounded-md resize-y w-full max-w-md"
+                                class="input input-bordered mt-5 rounded-md resize-y w-full max-w-md pt-3"


nitpick
mt-3 would be better

pt-3 is for the inner padding of the textarea, So that it looks cleaner, there is already a mt applied to it.

Datron · 2024-06-20T06:40:32Z

crates/frontend/src/components/default_config_form.rs

-
-                                        {config_value.get()}
-                                    </textarea>
+                let schema: Map<String, Value> = serde_json::from_value(config_schema_rs.get())


Should deciding input field based on schema type be a component of it's own? It is used across 3 files and should behave the same for all of them

pratikmishra356 · 2024-06-20T10:26:22Z

crates/frontend/src/components/dimension_form.rs

#[prop(default = 0)] priority: u16,
can you change default to something else , since we do not support priority 0
better is to change it to option
and if someone dont provide number then you should not let it submit

pratikmishra356 · 2024-06-20T10:32:40Z

crates/frontend/src/components/override_form.rs

@mahatoankitkumar can you fix this override button it should be aligned with context button

Didn't get you

ShubhranshuSanjeev · 2024-06-20T10:33:10Z

crates/frontend/src/components/default_config_form.rs

@mahatoankitkumar , we have similar fields for boolean and enum in both default_config and overrides form, can you make these a component and use them at both places, and also I think you should make similar changes for the context_form.

pratikmishra356 · 2024-06-26T11:34:45Z

crates/frontend/src/components/input_components.rs

+    #[prop(default = String::new())] class: String,
+    #[prop(default = false)] disabled: bool,
+) -> impl IntoView {
+    let mut enum_array: Vec<String> = vec![];


@mahatoankitkumar lets not assume enum array , as array of string only , it can be integer or decimal also
can we have that support , because if I am declaring enum of number in UI form dropdown shows nothing

sauraww · 2024-07-01T11:52:49Z

crates/frontend/src/components/context_form.rs

+                                    let schema: Map<String, Value> = serde_json::from_value(
+                                            dimensions_map.get(&dimension_label).unwrap().schema.clone(),
+                                        )
+                                        .unwrap();


Can we avoid doing plain unwrap here ?

sauraww · 2024-07-01T11:59:45Z

crates/frontend/src/components/default_config_form.rs

+                                class="input input-bordered w-full max-w-md"
+                                value=config_value.get()
+                                on:change=move |ev| {
+                                    logging::log!("{:?}", event_target_value(& ev));


Can we add the description here ?

sauraww · 2024-07-01T12:00:03Z

crates/frontend/src/components/default_config_form.rs

+                                placeholder="Value"
+                                class="input input-bordered w-full max-w-md pt-3"
+                                on:change=move |ev| {
+                                    logging::log!("{:?}", event_target_value(& ev));


Here as well

sauraww · 2024-07-01T12:03:47Z

crates/frontend/src/components/override_form.rs

+                .map(ConfigType::DefaultConfig)
+                .collect::<Vec<_>>(),
+        )
+        .expect("can't parse default config key")


Can we use unwrap_or instead and pass a json value having config_key_value :
unwrap_or(json!(config_key_value)

sauraww · 2024-07-01T13:19:30Z

crates/frontend/src/utils.rs

+        String::from("ENUM")
+    } else {
+        match schema.get("type").unwrap_or(&Value::Null) {
+            Value::String(str_) => str_.to_ascii_uppercase(),


Value::String(str) => str.to_ascii_uppercase(),

mahatoankitkumar self-assigned this Jun 19, 2024

mahatoankitkumar requested a review from a team as a code owner June 19, 2024 07:26

mahatoankitkumar linked an issue Jun 19, 2024 that may be closed by this pull request

Improve form inputs as per the types. Only for primitive types. #95

Closed

mahatoankitkumar removed a link to an issue Jun 19, 2024

Improve form inputs as per the types. Only for primitive types. #95

Closed

mahatoankitkumar linked an issue Jun 19, 2024 that may be closed by this pull request

Improve form inputs based on json schema type #51

Closed

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from d6178fa to 9d81ad6 Compare June 19, 2024 12:49

Datron requested changes Jun 20, 2024

View reviewed changes

ShubhranshuSanjeev added the priority Things that need immediate attention label Jun 20, 2024

pratikmishra356 reviewed Jun 20, 2024

View reviewed changes

ShubhranshuSanjeev requested changes Jun 20, 2024

View reviewed changes

mahatoankitkumar force-pushed the fix/improve/form/inputs branch 2 times, most recently from 9c5b201 to 6b28ba0 Compare June 21, 2024 06:37

Datron self-requested a review June 21, 2024 12:33

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from 05cd58b to bb8146e Compare June 24, 2024 08:54

mahatoankitkumar requested review from pratikmishra356 and ShubhranshuSanjeev June 24, 2024 08:54

Datron force-pushed the main branch from 0ec0b5f to 5610b93 Compare June 24, 2024 09:04

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from bb8146e to 43131f9 Compare June 25, 2024 06:11

Datron approved these changes Jun 25, 2024

View reviewed changes

mahatoankitkumar force-pushed the fix/improve/form/inputs branch 2 times, most recently from b155e44 to 029b79b Compare June 25, 2024 15:54

pratikmishra356 reviewed Jun 26, 2024

View reviewed changes

Datron self-requested a review June 27, 2024 05:36

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from 029b79b to 4afa622 Compare June 27, 2024 06:41

mahatoankitkumar requested a review from pratikmishra356 June 27, 2024 06:44

sauraww requested changes Jul 1, 2024

View reviewed changes

sauraww self-requested a review July 1, 2024 12:07

sauraww requested changes Jul 1, 2024

View reviewed changes

sauraww self-requested a review July 3, 2024 07:14

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from 4afa622 to 9417580 Compare July 3, 2024 12:09

fix: Improve form inputs

69d788b

mahatoankitkumar force-pushed the fix/improve/form/inputs branch from 2d3e17b to 69d788b Compare July 10, 2024 06:36

mahatoankitkumar closed this Jul 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Improve form inputs #129

fix: Improve form inputs #129

mahatoankitkumar commented Jun 19, 2024

Datron left a comment

Datron Jun 20, 2024

mahatoankitkumar Jun 20, 2024

Datron Jun 20, 2024

mahatoankitkumar Jun 20, 2024

pratikmishra356 Jun 20, 2024

pratikmishra356 Jun 20, 2024

mahatoankitkumar Jun 20, 2024

ShubhranshuSanjeev Jun 20, 2024

pratikmishra356 Jun 26, 2024

sauraww Jul 1, 2024

sauraww Jul 1, 2024

sauraww Jul 1, 2024

sauraww Jul 1, 2024

sauraww Jul 1, 2024

fix: Improve form inputs #129

fix: Improve form inputs #129

Conversation

mahatoankitkumar commented Jun 19, 2024

Problem

Datron left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment