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.

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

Modify the UI of events log switch tab #8286

Open
3 tasks
nihal467 opened this issue Aug 11, 2024 · 18 comments · May be fixed by #8825
Open
3 tasks

Modify the UI of events log switch tab #8286

nihal467 opened this issue Aug 11, 2024 · 18 comments · May be fixed by #8825

Comments

@nihal467
Copy link
Member

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

  • keep the daily round & events and filter button to a single row
  • Remove the sort-by button in the events green border to make it similar to the filter button in daily rounds
  • When the sort by is active in events, make it the primary color to show sort by is active

Screenshots

image
image

@JOSHIK27
Copy link
Contributor

Screen.Recording.2024-08-22.at.12.mp4

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
Copy link
Member

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

@JOSHIK27
Copy link
Contributor

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

Screen.Recording.2024-08-22.at.13.mp4

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

@github-actions github-actions bot added the stale label Sep 6, 2024
@rithviknishad
Copy link
Member

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
Copy link
Contributor

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.

Screen.Recording.2024-09-20.at.09.31.52.mov

@rithviknishad
Copy link
Member

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
Copy link
Contributor

JOSHIK27 commented Sep 20, 2024

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.

Screen.Recording.2024-09-20.at.14.mp4

@rithviknishad
Copy link
Member

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

@rithviknishad
Copy link
Member

rithviknishad commented Sep 20, 2024

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

@rithviknishad
Copy link
Member

Hey @JOSHIK27

Any updates on this? Stuck somewhere?

@JOSHIK27
Copy link
Contributor

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
Copy link
Member

My bad, it's SelectFormField

@JOSHIK27
Copy link
Contributor

JOSHIK27 commented Sep 30, 2024

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

Screen.Recording.2024-10-01.at.17.mp4

@bodhish
Copy link
Member

bodhish commented Oct 18, 2024

@nihal467 whats the status here? Also link the PR

@nihal467
Copy link
Member Author

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

@rithviknishad
Copy link
Member

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

@JOSHIK27
Copy link
Contributor

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

Sure. Will create the pr.

@JOSHIK27
Copy link
Contributor

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

@JOSHIK27 JOSHIK27 linked a pull request Oct 18, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants