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

Use shadcn Tooltip Components across FacilityCard #9317

Merged
merged 26 commits into from
Dec 23, 2024
Merged

Use shadcn Tooltip Components across FacilityCard #9317

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
4989af2
add tooltip for small screen
rajku-dev Dec 1, 2024
5e0c835
Merge branch 'ohcnetwork:develop' into tooltip-facility
rajku-dev Dec 6, 2024
72b3bff
edit en.json
rajku-dev Dec 6, 2024
a796de0
switch to Button
rajku-dev Dec 7, 2024
abcdbf9
edit outline
rajku-dev Dec 7, 2024
9eed329
fix breakpoint
rajku-dev Dec 7, 2024
27c9a40
remove ButtonV2 import
rajku-dev Dec 7, 2024
6033772
fixed the lint
nihal467 Dec 10, 2024
458de15
wrap Link with button
rajku-dev Dec 13, 2024
ac3d465
fix button
rajku-dev Dec 13, 2024
39deb1f
use shadcn tooltip
rajku-dev Dec 14, 2024
adcd8d4
increase tootltip font size and decrease delayDuration
rajku-dev Dec 14, 2024
a1780a1
format with prettier
rajku-dev Dec 16, 2024
7d949cb
correct facility spelling
rajku-dev Dec 16, 2024
bc220c9
Merge branch 'develop' into issue/9315/facilitycard-tooltip
rajku-dev Dec 17, 2024
6d34778
use TooltipComponent throughout
rajku-dev Dec 18, 2024
367c512
Merge branch 'develop' into issue/9315/facilitycard-tooltip
nihal467 Dec 18, 2024
65f9a29
Merge branch 'develop' into issue/9315/facilitycard-tooltip
rajku-dev Dec 19, 2024
2e716d5
Merge branch 'develop' into issue/9315/facilitycard-tooltip
nihal467 Dec 19, 2024
1ef0c1b
Merge branch 'develop' into issue/9315/facilitycard-tooltip
nihal467 Dec 20, 2024
1e16c1b
Merge branch 'develop' into issue/9315/facilitycard-tooltip
rajku-dev Dec 21, 2024
8c8684c
Merge branch 'develop' into issue/9315/facilitycard-tooltip
rajku-dev Dec 21, 2024
263dda9
Merge branch 'develop' into issue/9315/facilitycard-tooltip
nihal467 Dec 23, 2024
5c7d36f
Merge branch 'develop' into issue/9315/facilitycard-tooltip
nihal467 Dec 23, 2024
8123404
revert the view cns button id
nihal467 Dec 23, 2024
827b1ca
fixed the icon and text gap
nihal467 Dec 23, 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
3 changes: 2 additions & 1 deletion public/locale/en.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -991,6 +991,7 @@
"notification_cancelled": "Notification cancelled",
"notification_permission_denied": "Notification permission denied",
"notification_permission_granted": "Notification permission granted",
"notify": "Notify",
"number_of_aged_dependents": "Number of Aged Dependents (Above 60)",
"number_of_beds": "Number of beds",
"number_of_beds_out_of_range_error": "Number of beds cannot be greater than 100",
Expand Down Expand Up @@ -1420,7 +1421,7 @@
"view_cns": "View CNS",
"view_consultation_and_log_updates": "View Consultation / Log Updates",
"view_details": "View Details",
"view_faciliy": "View Facility",
"view_facility": "View Facility",
rajku-dev marked this conversation as resolved.
Show resolved Hide resolved
"view_files": "View Files",
"view_patients": "View Patients",
"view_update_patient_files": "View/Update patient files",
Expand Down
65 changes: 38 additions & 27 deletions src/components/Facility/FacilityCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import careConfig from "@careConfig";
import { Link } from "raviger";
import { Link,navigate } from "raviger";

Check failure on line 2 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Insert `·`
import { useState } from "react";
import { useTranslation } from "react-i18next";

Check failure on line 4 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Delete `";⏎import·{·Button·}·from·"@/components/ui/button`
import { Button } from "@/components/ui/button";

import Chip from "@/CAREUI/display/Chip";
import CareIcon from "@/CAREUI/icons/CareIcon";

Check failure on line 8 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Insert `on";⏎⏎import·{·Button·}·from·"@/components/ui/butt`

import { Avatar } from "@/components/Common/Avatar";
import ButtonV2, { Cancel, Submit } from "@/components/Common/ButtonV2";
import { Cancel, Submit } from "@/components/Common/ButtonV2";
import DialogModal from "@/components/Common/Dialog";
import { FacilityModel } from "@/components/Facility/models";
import TextAreaFormField from "@/components/Form/FormFields/TextAreaFormField";
Expand Down Expand Up @@ -112,19 +113,18 @@
</dt>
</div>
</div>
<ButtonV2
<Button
id="view-cns-button"
href={`/facility/${facility.id}/cns`}
border
ghost
onClick={() => navigate(`/facility/${facility.id}/cns`)}
variant={"outline_primary"}
className="mt-2 sm:mt-0"
>
<CareIcon
icon="l-monitor-heart-rate"
className="text-lg"
/>
<span>{t("view_cns")}</span>
</ButtonV2>
</Button>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add accessibility attributes and fix variant syntax

The View CNS button should follow the same accessibility pattern as other buttons in the component.

 <Button
   id="view-cns-button"
   onClick={() => navigate(`/facility/${facility.id}/cns`)}
-  variant={"outline_primary"}
+  variant="outline-primary"
   className="mt-2 sm:mt-0"
+  aria-label={t("view_cns")}
+  role="button"
 >
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
id="view-cns-button"
href={`/facility/${facility.id}/cns`}
border
ghost
onClick={() => navigate(`/facility/${facility.id}/cns`)}
variant={"outline_primary"}
className="mt-2 sm:mt-0"
>
<CareIcon
icon="l-monitor-heart-rate"
className="text-lg"
/>
<span>{t("view_cns")}</span>
</ButtonV2>
</Button>
<Button
id="view-cns-button"
onClick={() => navigate(`/facility/${facility.id}/cns`)}
variant="outline-primary"
className="mt-2 sm:mt-0"
aria-label={t("view_cns")}
role="button"
>
<CareIcon
icon="l-monitor-heart-rate"
className="text-lg"
/>
<span>{t("view_cns")}</span>
</Button>

</div>

<div className="mt-2 flex flex-wrap gap-1">
Expand Down Expand Up @@ -222,38 +222,49 @@
</div>
<div className="flex flex-wrap gap-2">
{["DistrictAdmin", "StateAdmin"].includes(userType) && (
<ButtonV2
<Button
id="facility-notify"
ghost
border
className="h-[38px]"
variant={"outline_primary"}
Jacobjeevan marked this conversation as resolved.
Show resolved Hide resolved
className="h-[38px] tooltip"
onClick={(_) => setNotifyModalFor(facility.id)}
aria-label={t("notify")}
role="button"
>
<CareIcon icon="l-megaphone" className="text-lg" />
<span className="hidden md:block">Notify</span>
</ButtonV2>
<span className="hidden sm:inline">

Check failure on line 234 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Replace `⏎··························{t("notify")}⏎························` with `{t("notify")}`
{t("notify")}
</span>
<span className="tooltip-text tooltip-top sm:hidden">
Copy link
Member

Choose a reason for hiding this comment

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

I'd recommend using shadcn's tooltip component as we are moving towards migrating to those in newer codebases.

Copy link
Contributor Author

@rajku-dev rajku-dev Dec 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@rithviknishad UI after applying shadcn Tooltip component

Recording.2024-12-14.110409.mp4

{t("notify")}
</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Migrate to shadcn's Tooltip component

As suggested in the previous review, we should use shadcn's Tooltip component for consistency with the new UI library.

+import { Tooltip, TooltipContent, TooltipTrigger } from "@/components/ui/tooltip"

-<span className="tooltip-text tooltip-top sm:hidden">
-  {t("notify")}
-</span>
+<Tooltip>
+  <TooltipTrigger asChild>
+    <span className="sm:hidden">{t("notify")}</span>
+  </TooltipTrigger>
+  <TooltipContent>
+    {t("notify")}
+  </TooltipContent>
+</Tooltip>

Apply similar changes to all tooltip implementations in the file.

Also applies to: 255-257

</Button>
)}
<ButtonV2
href={`/facility/${facility.id}`}
<Button
onClick={() => navigate(`/facility/${facility.id}`)}
id="facility-details"
rajku-dev marked this conversation as resolved.
Show resolved Hide resolved
border
ghost
className="h-[38px]"
variant={"outline_primary"}
className="tooltip h-[38px]"
aria-label={t("view_facility")}
role="button"
>
<CareIcon icon="l-hospital" className="text-lg" />
<span className="hidden md:inline">
{t("view_faciliy")}
<span className="hidden sm:inline">
{t("view_facility")}
</span>
<span className="tooltip-text tooltip-top sm:hidden">
{t("view_facility")}
</span>
</ButtonV2>
<ButtonV2
href={`/patients?facility=${facility.id}`}
</Button>
<Button
onClick={() => navigate(`/patients?facility=${facility.id}`)}

Check failure on line 259 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Replace `·navigate(`/patients?facility=${facility.id}`)` with `⏎························navigate(`/patients?facility=${facility.id}`)⏎······················`
id="facility-patients"
border
ghost
variant={"outline_primary"}
aria-label={t("view_patients")}
role="button"
>
<CareIcon icon="l-user-injured" className="text-lg" />
{t("view_patients")}
</ButtonV2>
{t("view_patients")}

Check failure on line 266 in src/components/Facility/FacilityCard.tsx

View workflow job for this annotation

GitHub Actions / lint 

Delete `··`
</Button>
{/* </div> */}
</div>
</div>
Expand Down
Loading