Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: using HtmlDisplay trait to show json in html #245

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: using HtmlDisplay trait to show json in html #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc3ddbe
fix: focusing on dropdown search input on open
ShubhranshuSanjeev Sep 23, 2024
609514b
fix: using HtmlDisplay trait to show json in html
ShubhranshuSanjeev Sep 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions crates/frontend/src/components/context_card.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@ pub fn context_card(
.clone()
.into_iter()
.map(|(k, v)| {
let k = Value::String(k.trim_matches('"').to_string());
let v = Value::String(format!("{}", v).trim_matches('"').to_string());
Map::from_iter(vec![(String::from("KEY"), k), (String::from("VALUE"), v)])
Map::from_iter(vec![
(String::from("KEY"), Value::String(k.clone())),
(String::from("VALUE"), v),
])
Comment on lines +34 to +37
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was this tested? Or can we remove this because of @sauraww 's change earlier?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tested, I am just getting rid of trim logic, as html_display will take care of showing these values without quotes in frontend.

})
.collect::<Vec<Map<String, Value>>>();

Expand Down
16 changes: 14 additions & 2 deletions crates/frontend/src/components/dropdown.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ where
DropdownBtnType::Select => "select select-bordered w-[28rem] items-center",
};

let node_ref = create_node_ref::<html::Input>();

view! {
<div
id=id
Expand All @@ -66,7 +68,16 @@ where
class=("dropdown-top", dropdown_direction == DropdownDirection::Top)
class=("dropdown-down", dropdown_direction == DropdownDirection::Down)
>
<label tabindex="0" class=format!("{} {}", class, btn_class)>
<label
tabindex="0"
class=format!("{} {}", class, btn_class)
on:click:undelegated=move |_| {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, how was this supposed to work ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get your question

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was asking how was this change helping in solving the autofocus issue, couldn't understand the logic by looking at the code

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In terms of implementation, I have an input element ref, every click on the button, I am using the input element ref to focus on the input box.

Can you specify what exactly you didn't understand?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To further add on this, the component library we are using keeps the dropdown mounted on the DOM, so autofocus do focus on the input box but since we make other interactions that focus is lost.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other way of doing this can be taking over the logic to display dropdown using signals, but I don't want to manually manage close for any click outside of dropdown.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeps the dropdown mounted on the DOM

why is it always mounted ?
shouldn't we just unmount it when not in use
anyways, the behaviour which we simply wanted is the moment it becomes visible (gets mounted in ideal case), the input component should get focused

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we just unmount it when not in use

Ideally yes.

why is it always mounted ?

That's how daisy UI does it.
It just changes the visibility to hidden and opacity to 0.

if let Some(element) = node_ref.get() {
let _ = element.focus();
}
}
>

<i class=format!("{dropdown_icon}")></i>
{dropdown_text}
</label>
Expand All @@ -86,7 +97,8 @@ where
type="text"
class="grow"
placeholder="Search"
name=name
ref_=node_ref
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wont this suffice ?

Suggested change
ref_=node_ref
autofocus={true}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That wasn't working, so went ahead with this approach.

name=name.clone()
value=search_term.get_untracked()
on:input=move |event| {
set_search_term.set(event_target_value(&event));
Expand Down
Loading
Loading