feat: Added grouping in default_config page #9

mahatoankitkumar · 2024-04-23T06:49:02Z

Problem

Add grouping in default_config page

Solution

Added grouping in default_config page to show keys in folder structure.

Environment variable changes

N.A.

Pre-deployment activity

N.A.

Post-deployment activity

N.A.

API changes

N.A.

Possible Issues in the future

N.A.

Datron · 2024-04-23T10:58:54Z

@mahatoankitkumar can you add screenshots?

Datron · 2024-04-23T11:00:02Z

crates/frontend/src/components/default_config_form/default_config_form.rs

-        let f_name = config_key.get();
+        let f_name = prefix
+            .clone()
+            .map_or_else(|| config_key.get(), |p| p + &config_key.get());


What does |p| p + &config_key.get()
Supposed to do?

Adds prefix to the key. Will change p to prefix more readibility.

Datron · 2024-04-23T11:01:23Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

+
+    create_effect(move |_| {
+        let query_params_map = query_params.try_get();
+        if let Some(query_map) = query_params_map {


could we simplify this block of code and reduce the ifs?

ShubhranshuSanjeev · 2024-04-23T07:14:45Z

crates/frontend/src/components/default_config_form/default_config_form.rs

@@ -23,6 +23,7 @@ pub fn default_config_form<NF>(
    #[prop(default = String::new())] config_pattern: String,
    #[prop(default = String::new())] config_value: String,
    #[prop(default = None)] function_name: Option<Value>,
+    #[prop(default = None)] prefix: Option<String>,


Suggested change

#[prop(default = None)] prefix: Option<String>,

#[prop(default = String::new())] prefix: String,

Optional parameter makes more sense here.

ShubhranshuSanjeev · 2024-04-23T07:18:06Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

@mahatoankitkumar , can you make the folder and the file name default_config also the component function name default_config?

ShubhranshuSanjeev · 2024-04-23T07:21:40Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

+    create_effect(move |_| {
+        let query_params_map = query_params.try_get();
+        if let Some(query_map) = query_params_map {
+            let opt_prefix = query_map.get("prefix");


Suggested change

let opt_prefix = query_map.get("prefix");

filters.set(query_map.get("prefix").cloned());

we can directly go with this, right?

ShubhranshuSanjeev · 2024-04-23T07:47:46Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

+
+        let expand = move |_: &str, row: &Map<String, Value>| {
+            let key_name = row["key"].clone().to_string().replace("\"", "");
+            let key_name_copy = key_name.clone();


Suggested change

let key_name_copy = key_name.clone();

let label = key_name.clone();

Since this is used as the cell value on the page.

ShubhranshuSanjeev · 2024-04-23T10:00:32Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs


    let table_columns = create_memo(move |_| {
        let edit_col_formatter = move |_: &str, row: &Map<String, Value>| {
            logging::log!("{:?}", row);
            let row_key = row["key"].clone().to_string().replace("\"", "");
+            let key = row_key.clone();


Suggested change

let key = row_key.clone();

let is_folder = row_key.contains(".");

ShubhranshuSanjeev · 2024-04-23T10:00:46Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

@@ -116,10 +144,36 @@ pub fn DefaultConfig() -> impl IntoView {
            let edit_icon: HtmlElement<html::I> =
                view! { <i class="ri-pencil-line ri-xl text-blue-500"></i> };

-            view! { <span class="cursor-pointer" on:click=edit_click_handler>{edit_icon}</span> }.into_view()
+            if key.to_owned().contains(".") {


Suggested change

if key.to_owned().contains(".") {

if is_folder {

ShubhranshuSanjeev · 2024-04-23T10:13:30Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

+                            }
+                            folder_click_handler(Some(key.clone()))
+                        }>
+                            {key_name_copy.clone()}


Suggested change

{key_name_copy.clone()}

{label}

.clone() is not needed

ShubhranshuSanjeev · 2024-04-23T11:10:54Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

+                            vec!["".to_string(), key.to_string()]
+                        };
+
+                        if let Some(filtered_key) = key_arr.get(1) {


@mahatoankitkumar , can you explain the approach here to generate a key name?

ShubhranshuSanjeev · 2024-04-23T11:17:19Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

@@ -30,11 +32,37 @@ pub fn DefaultConfig() -> impl IntoView {
    );

    let selected_config = create_rw_signal::<Option<RowData>>(None);
+    let filters = create_rw_signal::<Option<String>>(None);


Suggested change

let filters = create_rw_signal::<Option<String>>(None);

let key_prefix = create_rw_signal::<Option<String>>(None);

ShubhranshuSanjeev · 2024-04-23T12:31:40Z

crates/frontend/src/pages/DefaultConfig/DefaultConfig.rs

-                                    </DrawerBtn>
+                                <div class="flex justify-between pb-2">
+                                    <div class="flex justify-between pt-3">{
+                                        {move || {


Suggested change

{move || {

```suggestion

{

move || {

let data = bread_crums.get();

let last_index = data.len() - 1;

data.iter()

.enumerate()

.map(|(index, item)| {

let value = item.value.clone();

let is_link = item.is_link.clone();

let is_last_item = index < last_index;

let item_classname = if item.is_link {

"cursor-pointer text-blue-500 underline underline-offset-2"

} else {

""

};

view! {

<div class="flex">

<h2

on:click=move |_| {

if is_link {

folder_click_handler(value.clone())

}

}

class={item_classname}

>

{item.key.clone()}

</h2>

<h2 class="pl-4 pr-4">

{if is_last_item { ">" } else { "" }}

</h2>

</div>

}

})

.collect_view()

}

}

We can use derive signal like this, to solution this out

let bread_crums = Signal::derive(move || { let filters = filters.get(); let mut default_bread_crums = vec![BreadCrums { key: "Default Config".to_string(), value: None, is_link: true, }]; let mut bread_crums = match filters { Some(prefix) => { let partials = prefix .trim_matches('.') .split(".") .map(str::to_string) .collect::<Vec<String>>(); partials .into_iter() .fold(String::new(), |mut prefix, partial| { default_bread_crums.push(BreadCrums { key: partial.clone(), value: Some(prefix.clone()), is_link: true, }); prefix.push_str(&partial); prefix.push('.'); prefix }); default_bread_crums } None => default_bread_crums, }; if let Some(last_crum) = bread_crums.last_mut() { last_crum.is_link = false; } bread_crums });

sure will try this.

Datron · 2024-04-24T06:07:33Z

crates/frontend/src/pages/default_config/mod.rs

@@ -0,0 +1 @@
+pub mod default_config;


Can we stop doing these one line mod.rs files? We can just put all the code here

Leave it as is for now

Datron · 2024-04-24T06:08:50Z

crates/experimentation_platform/src/db/models.rs

+    Deserialize,
+    Serialize,
+    diesel_derive_enum::DbEnum,
+    QueryId,


@mahatoankitkumar you can get rid of QueryId here, I was trying to solve something and added it by mistake.

pratikmishra356 · 2024-04-24T06:42:53Z

crates/frontend/src/pages/default_config/default_config.rs

+            is_link: true,
+        }];
+
+        let mut bread_crums = match key_prefix {


can we have different variable name bread_crums ?

bread_crums name serves the purpose.

ShubhranshuSanjeev · 2024-04-26T06:41:37Z

crates/frontend/src/pages/default_config/default_config.rs

+                                        <div class="flex">
+                                            <label on:click=move |_| {
+                                                folder_click_handler(None);
+                                                enable_grouping.set(!enable_grouping.get());}


Suggested change

enable_grouping.set(!enable_grouping.get());}

enable_grouping.update(|prev| *prev = !(*prev));

This isn't re-rendering the component.

mahatoankitkumar self-assigned this Apr 23, 2024

mahatoankitkumar requested a review from a team as a code owner April 23, 2024 06:49

Datron force-pushed the main branch from 32863bc to 4d83310 Compare April 23, 2024 06:50

mahatoankitkumar force-pushed the feat/grouping/default_config branch from 701d931 to a45be66 Compare April 23, 2024 06:50

Datron force-pushed the main branch 5 times, most recently from 7dcb3c0 to 3b7e262 Compare April 23, 2024 10:03

mahatoankitkumar force-pushed the feat/grouping/default_config branch from a45be66 to 875e51c Compare April 23, 2024 12:10

Datron requested changes Apr 23, 2024

View reviewed changes

mahatoankitkumar force-pushed the feat/grouping/default_config branch from 875e51c to c558e53 Compare April 23, 2024 12:25

ShubhranshuSanjeev requested changes Apr 23, 2024

View reviewed changes

mahatoankitkumar force-pushed the feat/grouping/default_config branch 2 times, most recently from a20f321 to 5d1d379 Compare April 23, 2024 18:46

Datron reviewed Apr 24, 2024

View reviewed changes

mahatoankitkumar force-pushed the feat/grouping/default_config branch 2 times, most recently from d4f392a to 1264fbf Compare April 24, 2024 06:16

Datron requested review from Datron and ShubhranshuSanjeev April 24, 2024 06:28

Datron approved these changes Apr 24, 2024

View reviewed changes

Datron force-pushed the feat/grouping/default_config branch from 1264fbf to 86cc900 Compare April 24, 2024 06:29

pratikmishra356 reviewed Apr 24, 2024

View reviewed changes

mahatoankitkumar force-pushed the feat/grouping/default_config branch 2 times, most recently from ac68951 to fed78c9 Compare April 26, 2024 06:36

ShubhranshuSanjeev requested changes Apr 26, 2024

View reviewed changes

feat: Added grouping in default_config page

691556a

mahatoankitkumar force-pushed the feat/grouping/default_config branch from fed78c9 to 691556a Compare April 26, 2024 08:00

ShubhranshuSanjeev approved these changes Apr 26, 2024

View reviewed changes

ShubhranshuSanjeev merged commit 5ac099d into main Apr 26, 2024
2 checks passed

Datron pushed a commit that referenced this pull request May 3, 2024

feat: Added grouping in default_config page (#9)

9f4a46a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added grouping in default_config page #9

feat: Added grouping in default_config page #9

mahatoankitkumar commented Apr 23, 2024 •

edited

Loading

Datron commented Apr 23, 2024

Datron Apr 23, 2024

mahatoankitkumar Apr 23, 2024

Datron Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

mahatoankitkumar Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

ShubhranshuSanjeev Apr 23, 2024

mahatoankitkumar Apr 23, 2024

Datron Apr 24, 2024

Datron Apr 24, 2024

Datron Apr 24, 2024

pratikmishra356 Apr 24, 2024

mahatoankitkumar Apr 24, 2024

ShubhranshuSanjeev Apr 26, 2024

mahatoankitkumar Apr 26, 2024

	#[prop(default = None)] prefix: Option<String>,
	#[prop(default = String::new())] prefix: String,

	let opt_prefix = query_map.get("prefix");
	filters.set(query_map.get("prefix").cloned());

	let key_name_copy = key_name.clone();
	let label = key_name.clone();

	let key = row_key.clone();
	let is_folder = row_key.contains(".");

	let filters = create_rw_signal::<Option<String>>(None);
	let key_prefix = create_rw_signal::<Option<String>>(None);

-                                        {move || {
+```suggestion
+                                        {
+                                            move || {
+                                                let data = bread_crums.get();
+                                                let last_index = data.len() - 1;
+                                                data.iter()
+                                                    .enumerate()
+                                                    .map(|(index, item)| {
+                                                        let value = item.value.clone();
+                                                        let is_link = item.is_link.clone();
+                                                        let is_last_item = index < last_index;
+                                                        let item_classname = if item.is_link {
+                                                            "cursor-pointer text-blue-500 underline underline-offset-2"
+                                                        } else {
+                                                            ""
+                                                        };
+                                                        view! {
+                                                            <div class="flex">
+                                                                <h2
+                                                                    on:click=move |_| {
+                                                                        if is_link {
+                                                                            folder_click_handler(value.clone())
+                                                                        }
+                                                                    }
+                                                                    class={item_classname}
+                                                                >
+                                                                    {item.key.clone()}
+                                                                </h2>
+                                                                <h2 class="pl-4 pr-4">
+                                                                    {if is_last_item { ">" } else { "" }}
+                                                                </h2>
+                                                            </div>
+                                                        }
+                                                    })
+                                                    .collect_view()
+                                            }
+                                        }

	enable_grouping.set(!enable_grouping.get());}
	enable_grouping.update(\|prev\| prev = !(prev));

feat: Added grouping in default_config page #9

feat: Added grouping in default_config page #9

Conversation

mahatoankitkumar commented Apr 23, 2024 • edited Loading

Problem

Solution

Environment variable changes

Pre-deployment activity

Post-deployment activity

API changes

Possible Issues in the future

Datron commented Apr 23, 2024

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

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

mahatoankitkumar commented Apr 23, 2024 •

edited

Loading