ohcnetwork / care_fe

Care is a Digital Public Good enabling TeleICU & Decentralised Administration of Healthcare Capacity across States.
https://care.ohc.network
MIT License
249 stars 432 forks source link

Modify the UI of events log switch tab #8286

Closed nihal467 closed 2 weeks ago

nihal467 commented 3 months ago

Describe the bug

Currently the switch tab of events & daily road and filters button in the consultation page are utilizing two separate rows, unnecessarily

To Reproduce Steps to reproduce the behavior:

  1. Go to the patient tab
  2. click on a active consultation
  3. See error

Expected behavior

Screenshots

image image

JOSHIK27 commented 2 months ago

https://github.com/user-attachments/assets/42e51659-108f-4f5f-81bb-ce4e391b7ce5

I have brought them to single row by creating filter ui elements in the parent component (removed the logic related to them in child components where they were present initially) and passed the corresponding query values of both filter ui as props to components which render the result obtained from filtering. Adjusted the grid to 3 columns. Please let me know if this is what you wanted ? and happy to modify if anything @rithviknishad

rithviknishad commented 2 months ago

The "sort by" and "filter" is NOT a tab right? Can we keep it outside the tab switch?

JOSHIK27 commented 2 months ago

The "sort by" and "filter" is NOT a tab right? Can we keep it outside the tab switch?

https://github.com/user-attachments/assets/9a32ff26-1b2f-42e5-a1ed-f6ff424a4851

There is some inconsistency in the appearance of filter buttons. How do we make it better ? any inputs @rithviknishad

rithviknishad commented 1 month ago

We could use a Popover with 3-dot menu or filters icon for both. And keep the sort by as an option (select menu) in the popover instead.

JOSHIK27 commented 1 month ago

We could use a Popover with 3-dot menu or filters icon for both. And keep the sort by as an option (select menu) in the popover instead.

is this how you wanted @rithviknishad ? Let me know if any changes required.

https://github.com/user-attachments/assets/d098d94f-47b3-4176-a166-bb1a0b8dc4b3

rithviknishad commented 1 month ago

This seems to be nested popovers which was not what I intended.

Let's inline the filter contents to the first popover itself.

Also could you make the 3-dot menu to use to <ButtonV2>? The current button doesn't seem to be clickable.

image
JOSHIK27 commented 1 month ago

This seems to be nested popovers which was not what I intended.

Let's inline the filter contents to the first popover itself.

Also could you make the 3-dot menu to use to <ButtonV2>? The current button doesn't seem to be clickable.

image

Sure ill change.

Update: @rithviknishad As both filter ui components are different (popover, dropdown), it was straight forward to achieve the requirement for popover botton since we are creating a popover (3 dot). Inorder to have same appearence for both i had to change core menu component (not custom dropdownmenu component) . It will affect in other places as it is imported. So, i have pass label of menu as "None" and conditionally rendered 3 dot button to maintain same appearence.

Let me know what you think.

https://github.com/user-attachments/assets/d57a83a4-af8e-4564-afb9-06da08fb2122

rithviknishad commented 1 month ago

Checkout SelectMenuFormField. Lets use Popover for both sort by and filter. The sort by popover can contain SelectMenuFormField as it's child.

rithviknishad commented 1 month ago

Also let the 3-dot button's variant be "secondary", also ensure it's square. Would be difficult to click if so slim.

rithviknishad commented 1 month ago

Hey @JOSHIK27

Any updates on this? Stuck somewhere?

JOSHIK27 commented 1 month ago

Hey @JOSHIK27

Any updates on this? Stuck somewhere?

Apologies for the delay. I could not find SelectMenuFormField anywhere in neither headless ui elements nor the codebase. Checked with headlessui documentation. Am i missing something ? Could you please direct me with this element. I'll give u the update about pr eod.

rithviknishad commented 1 month ago

My bad, it's SelectFormField

JOSHIK27 commented 1 month ago

My bad, it's SelectFormField

thank you. Will look into it

@rithviknishad if we use selectformfield inside popover panel for sortbydropdown filter, would not that be a nested one as earlier ?

Update:

Instead of using selectformfield, i rendered all the options in the popover panel as divs with exact styling as selectformfield options for consistency. Now both filters are popvers. Please let me know if anything needs to be changed

https://github.com/user-attachments/assets/999de6e5-783d-4832-8ada-a61c64bdf259

bodhish commented 3 weeks ago

@nihal467 whats the status here? Also link the PR

nihal467 commented 3 weeks ago

@bodhish i think you mis tagged me, @rithviknishad @JOSHIK27 can you share the status on this issue and link the PR

rithviknishad commented 3 weeks ago

@JOSHIK27 the recording shared in the last comment looks fine to me

JOSHIK27 commented 3 weeks ago

@JOSHIK27 the recording shared in the last comment looks fine to me

Sure. Will create the pr.

JOSHIK27 commented 3 weeks ago

Here's the pr Please let me know if any thing needs to be changed. @rithviknishad