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

Fix: UI Issues in Data Fetch Scroll, Text Area Overflow, and User Profile Navigation in notes tab #10200

Closed
wants to merge 11 commits into from
Closed

Fix: UI Issues in Data Fetch Scroll, Text Area Overflow, and User Profile Navigation in notes tab #10200

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
05af782
Add user navigation from message items and enhance loading indicator …
shauryag2002 Jan 11, 2025
6a8c171
Smooth scroll to pages element on EncounterNotesTab mount
shauryag2002 Jan 11, 2025
a91e058
revert date
shauryag2002 Jan 11, 2025
21200d2
Add scroll-to-bottom functionality for new messages in EncounterNotesTab
shauryag2002 Jan 12, 2025
a78bf53
Remove unused smooth scroll effect for pages element in EncounterNote…
shauryag2002 Jan 12, 2025
4f57530
Merge branch 'develop' of https://github.com/shauryag2002/care_fe int…
shauryag2002 Jan 18, 2025
766d77a
Update EncounterNotesTab.tsx
shauryag2002 Jan 18, 2025
0aebf3f
Merge branch 'develop' of https://github.com/shauryag2002/care_fe int…
shauryag2002 Jan 26, 2025
fb66af5
Merge branch 'develop' of https://github.com/shauryag2002/care_fe int…
shauryag2002 Feb 1, 2025
e607df1
Merge branch 'develop' of https://github.com/shauryag2002/care_fe int…
shauryag2002 Feb 6, 2025
21d2a38
Remove unnecessary code
shauryag2002 Feb 6, 2025
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
12 changes: 12 additions & 0 deletions .unimportedrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,23 @@
"node_modules/find-cypress-specs/src/index.js"
]
],
[
"tsx/cjs",
[
"node_modules\\find-cypress-specs\\src\\index.js"
]
],
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

Remove duplicate path entries.

The added entries with backslashes are redundant as the existing entries with forward slashes (lines 33-38 and 46-50) already cover these paths. Modern path handling in Node.js and most build tools automatically normalize path separators.

-    [
-      "tsx/cjs",
-      [
-        "node_modules\\find-cypress-specs\\src\\index.js"
-      ]
-    ],
-    [
-      "virtual:pwa-register",
-      [
-        "src\\index.tsx"
-      ]
-    ]

Also applies to: 51-56

[
"virtual:pwa-register",
[
"src/index.tsx"
]
],
[
"virtual:pwa-register",
[
"src\\index.tsx"
]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These changes occurred due to running npx unimported -u.

Why?

The pre-commit hook failed due to unresolved imports detected by unimported, preventing the commit.

Root Cause

unimported flagged two unresolved imports:

  1. virtual:pwa-register in src/index.tsx
  2. tsx/cjs in node_modules/find-cypress-specs/src/index.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

❌ Unimported: Initializing

Summary (unimported v1.31.1)

Category Count
Entry Files 11
Unresolved Imports 2
Unused Dependencies 0
Unimported Files 0

📂 Entry Files

  1. src/index.tsx
  2. vite.config.mts
  3. tailwind.config.js
  4. cypress.config.ts
  5. scripts/
  6. scripts/generate-build-version.js
  7. scripts/generate-sbom-data.ts
  8. scripts/generate-supported-browsers.mjs
  9. scripts/install-platform-deps.ts
  10. scripts/setup-care-apps.ts
  11. scripts/sort-locales.js

⚠️ Unresolved Imports (2)

# Import Location
1 virtual:pwa-register src/index.tsx
2 tsx/cjs node_modules/find-cypress-specs/src/index.js

🔍 Suggested Fix

Inspect the results and run:

npx unimported -u

Copy link
Member

Choose a reason for hiding this comment

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

these imports are already ignored in the config. just the slashes are different.

we do not encourage using windows for development.
discard these changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, @rithviknishad

]
],
"respectGitignore": true,
Expand Down
37 changes: 31 additions & 6 deletions src/pages/Encounters/tabs/EncounterNotesTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
Send,
Users,
} from "lucide-react";
import { navigate } from "raviger";
import { useEffect, useRef, useState } from "react";
import { useTranslation } from "react-i18next";
import { useInView } from "react-intersection-observer";
Expand Down Expand Up @@ -48,6 +49,7 @@ import Loading from "@/components/Common/Loading";
import { CardListSkeleton } from "@/components/Common/SkeletonLoading";

import useAuthUser from "@/hooks/useAuthUser";
import useSlug from "@/hooks/useSlug";

import routes from "@/Utils/request/api";
import mutate from "@/Utils/request/mutate";
Expand Down Expand Up @@ -119,6 +121,11 @@ const ThreadItem = ({
const MessageItem = ({ message }: { message: Message }) => {
const authUser = useAuthUser();
const isCurrentUser = authUser?.external_id === message.created_by.id;
const facilityId = useSlug("facility");

const navigateToUser = () => {
navigate(`/facility/${facilityId}/users/${message.created_by.username}`);
};

return (
<div
Expand All @@ -136,7 +143,7 @@ const MessageItem = ({ message }: { message: Message }) => {
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<div className="flex">
<div className="flex cursor-pointer" onClick={navigateToUser}>
<Avatar
name={message.created_by.username}
imageUrl={message.created_by.profile_picture_url}
Expand All @@ -157,7 +164,10 @@ const MessageItem = ({ message }: { message: Message }) => {
)}
>
<p className="text-xs space-x-2 mb-1">
<span className="text-gray-700 font-medium">
<span
className="text-gray-700 font-medium cursor-pointer"
onClick={navigateToUser}
>
{message.created_by.username}
</span>
<time
Expand Down Expand Up @@ -306,6 +316,7 @@ export const EncounterNotesTab = ({ encounter }: EncounterTabProps) => {
const [isThreadsExpanded, setIsThreadsExpanded] = useState(false);
const [showNewThreadDialog, setShowNewThreadDialog] = useState(false);
const [newMessage, setNewMessage] = useState("");
const [scrollToBottom, setScrollToBottom] = useState(false);
const messagesEndRef = useRef<HTMLDivElement>(null);
const { ref, inView } = useInView();

Expand Down Expand Up @@ -384,6 +395,7 @@ export const EncounterNotesTab = ({ encounter }: EncounterTabProps) => {
setNewMessage("");
setTimeout(() => {
messagesEndRef.current?.scrollIntoView({ behavior: "smooth" });
setScrollToBottom(true);
}, 100);
},
});
Expand All @@ -397,8 +409,14 @@ export const EncounterNotesTab = ({ encounter }: EncounterTabProps) => {

// Scroll to bottom on initial load and thread change
useEffect(() => {
if (messagesData && !messagesLoading && !isFetchingNextPage) {
if (
Copy link
Contributor

@Jacobjeevan Jacobjeevan Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The issue with this approach is that it will just scroll to the top. Instead it should scroll to the very bottom of the newly loaded data, so navigating the conversation would be seamless for the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm currently stuck on implementing this properly. Right now, the scroll jumps to the top instead of smoothly moving to the bottom of the newly loaded data. Could you guide me on how to fix this in code and achieve the desired behavior?

Copy link
Contributor

Choose a reason for hiding this comment

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

I was looking into a few different approaches yesterday, but wasn't satisfied with the result 🤔 Let's remove the changes for scroll behavior for now, I'll take that up. Clear the merge conflicts so we can get this PR merged.

messagesData &&
!messagesLoading &&
!isFetchingNextPage &&
(messagesData.pages.length === 1 || scrollToBottom)
) {
messagesEndRef.current?.scrollIntoView();
setScrollToBottom(false);
}
}, [selectedThread, messagesData, messagesLoading, isFetchingNextPage]);

Expand Down Expand Up @@ -581,9 +599,15 @@ export const EncounterNotesTab = ({ encounter }: EncounterTabProps) => {
))
)}
{isFetchingNextPage && (
<div className="py-2">
<div className="space-y-4">
<CardListSkeleton count={3} />
<div className="flex justify-center absolute top-2 inset-x-0">
<div className="flex items-center space-x-2">
<div className="w-26 flex gap-2 px-2 py-1 bg-primary-200 rounded-md">
<div>{t("loading")}</div>
<Loader2
className="h-5 w-5 animate-spin top-0.5 relative"
size={8}
/>
</div>
</div>
</div>
)}
Expand All @@ -607,6 +631,7 @@ export const EncounterNotesTab = ({ encounter }: EncounterTabProps) => {
}
}
}}
className="max-h-[150px]"
/>
<Button
type="submit"
Expand Down
Loading