made a reusable component for table in routine and nursing care procedures #8608
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
|
| const SharedSectionTable: React.FC<SharedSectionTableProps> = ({ | ||
| data, | ||
| rows, | ||
| choices = {}, | ||
| translateKey, | ||
| t, | ||
| }) => { |
There was a problem hiding this comment.
- Rename this to be
LogUpdateAnalayseTablesince it's built for it. - use
useTranslationsinstead of passing it as a prop. - i18n suffix
LOG_UPDATE_FIELD_LABEL__is not going to change, so skip passing it as a prop.
| const SharedSectionTable: React.FC<SharedSectionTableProps> = ({ | |
| data, | |
| rows, | |
| choices = {}, | |
| translateKey, | |
| t, | |
| }) => { | |
| const LogUpdateAnalayseTable: React.FC<SharedSectionTableProps> = ({ | |
| data, | |
| rows, | |
| choices = {}, | |
| }) => { |
There was a problem hiding this comment.
Let's keep this in ConsultationNursingTab file itself. Keeping one section here and one section in a separate file isn't nice.
|
Hey @rithviknishad I am not able to find where the dates input for nursing care is . I think the format in which it is entered is giving invalid date. How can I change it? should i change the date format in the logUpdateAnalyseTable itself if it is in other format? 02:18 AM; "15/05/2024" to " 2024-09-20 09:24:23.859000+00:00 " |
|
Hey @rithviknishad I fixed the date issue and fixed the 1st column in in mobile view. |
Hey @rithviknishad This is how the page looks like , please tell me if I need to change anything.
|
|
👋 Hi, @Sahil-Sinha-11, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
@Sahil-Sinha-11 fix the lint issue |
|
LGTM @khavinshankar @rithviknishad @Sahil-Sinha-11 fix the lint issue |
nihal467
added
tested
and removed
needs testing
merge conflict
src/Components/Facility/Consultations/components/SharedTable.tsx
Outdated
Show resolved
Hide resolved
src/Components/Facility/Consultations/components/SharedTable.tsx
Outdated
Show resolved
Hide resolved
| const dateConversion = (str: string) => { | ||
| const time = str.split(";")[0].trim(); | ||
|
|
||
| const date = str.split(";")[1].trim(); | ||
|
|
||
| const dd = date.split("/")[0]; | ||
| const mm = date.split("/")[1]; | ||
|
|
||
| const yyyy = date.split("/")[2]; | ||
|
|
||
| return time + ";" + mm + "/" + dd + "/" + yyyy; | ||
| }; |
There was a problem hiding this comment.
we can use formatDateTime instead of this right?
There was a problem hiding this comment.
@rithviknishad actually formatDateTime uses days.js that expects date in format mm/dd/yyyy but the date format passed in nursingtab was different so I had to convert the format and then pass it to formatDateTime function
There was a problem hiding this comment.
Whats the format of time received? I am pretty sure you can transform a format to other super easily.
There was a problem hiding this comment.
From ChatGPT
In this case, you can utilize day.js to handle the date format transformation easily. Here's a suggestion for tackling the date formatting using day.js:
- You can parse the incoming date string using
day.js, and then transform it to the format required byformatDateTime.
Here’s a sample implementation for transforming the date format:
import dayjs from 'dayjs';
const dateConversion = (str: string) => {
const [time, date] = str.split(';').map(s => s.trim());
// Assuming the incoming date is in "dd/mm/yyyy" format
const parsedDate = dayjs(date, 'DD/MM/YYYY');
// Format the date to the required "mm/dd/yyyy" format for formatDateTime
const formattedDate = parsedDate.format('MM/DD/YYYY');
return `${time}; ${formattedDate}`;
};In this implementation:
- The
dateis first split and trimmed. - The
dayjs(date, 'DD/MM/YYYY')parses the input date, assuming the formatDD/MM/YYYY. - The
.format('MM/DD/YYYY')converts it into the requiredMM/DD/YYYYformat forformatDateTime.
You can now pass the result to formatDateTime easily after this conversion.
There was a problem hiding this comment.
backend is giving the data in standard format @Sahil-Sinha-11
However you are already formatting it when sending the data to the component after receiving it from the backend, and then deconstructing the formatted back to unformatted, just for format it again?
There was a problem hiding this comment.
@rithviknishad I am not able to run the application it shows a file does not exist( pluginMap). I even tried on devlop branch.
.
There was a problem hiding this comment.
There was a problem hiding this comment.
Hey @rithviknishad , I resolved it .
|
👋 Hi, @Sahil-Sinha-11, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
@nihal467 please check this PR and get it closed |
|
LGTM @rithviknishad @khavinshankar can you take over the PR and clear the merge conflict |
WalkthroughThe changes in this pull request involve significant enhancements to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (7)
src/Components/Facility/Consultations/LogUpdateAnalayseTable.tsx (4)
6-10: Consider improving type safety of the interface.The current interface could be more type-safe:
Record<string, any>allows any value type which could lead to runtime errors- The relationship between
titleandfieldin the rows array is not clearly typedConsider this improved interface:
interface RowDefinition { title?: string; field?: string; subField?: boolean; } interface SharedSectionTableProps<T = Record<string, string | boolean | null>> { data: Record<string, T>; rows: Array<RowDefinition>; choices?: Record<string, Record<number | string, string>>; }
24-26: Consider returning a consistent placeholder for null values.Returning a space string for null values might cause layout inconsistencies. Consider using the same "-" placeholder as other empty states.
if (value == null) { - return " "; + return "-"; }
31-35: Improve type safety of choices lookup.The type casting here is a bit unsafe and could be improved with better typing of the choices prop.
Consider using a more type-safe approach:
if (field && field in choices) { const choiceMap = choices[field]; const choice = value in choiceMap ? choiceMap[value] : undefined; return choice ? t(`${field.toUpperCase()}__${choice}`) : "-"; }
46-46: Add descriptive content to empty header cell.The empty header cell should have a descriptive label for better accessibility.
-<th className="sticky left-0 border-b-2 border-r-2 border-black bg-white"></th> +<th className="sticky left-0 border-b-2 border-r-2 border-black bg-white" aria-label={t('field_names_column')}> + {t('field_names_column')} +</th>src/Components/Facility/ConsultationDetails/ConsultationNursingTab.tsx (3)
136-145: Simplify array transformation usingflatMapConsider replacing the
mapandreducecombination withflatMapfor cleaner and more efficient code.Apply this diff:
-const dataToDisplay = data - .map((x) => - x.nursing - ? x.nursing.map((f: any) => { - f["date"] = x.date; - return f; - }) - : [], - ) - .reduce((accumulator, value) => accumulator.concat(value), []); +const dataToDisplay = data.flatMap((x) => + x.nursing + ? x.nursing.map((f: any) => { + f["date"] = x.date; + return f; + }) + : [], +);
138-141: Avoid mutating objects withinmapfunctionTo prevent unintended side effects, avoid mutating objects inside
map. Return a new object instead.Apply this diff:
- f["date"] = x.date; - return f; + return { + ...f, + date: x.date, + };
110-110: Simplify conditional statement using optional chainingYou can simplify the condition by utilizing optional chaining for cleaner code.
Apply this diff:
-if (res && res.ok && data) { +if (res?.ok && data) {🧰 Tools
🪛 Biome
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/Components/Facility/ConsultationDetails/ConsultationNursingTab.tsx(4 hunks)src/Components/Facility/Consultations/LogUpdateAnalayseTable.tsx(1 hunks)src/Components/Facility/Consultations/NursingPlot.tsx(0 hunks)
💤 Files with no reviewable changes (1)
- src/Components/Facility/Consultations/NursingPlot.tsx
🧰 Additional context used
🪛 Biome
src/Components/Facility/ConsultationDetails/ConsultationNursingTab.tsx
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
src/Components/Facility/Consultations/LogUpdateAnalayseTable.tsx (2)
41-92: Implementation successfully meets PR objectives.
The component successfully implements all main requirements:
- Locked header row using sticky positioning
- Fixed width columns using Tailwind classes
- Reusable component structure with proper props interface
The table implementation is clean and maintainable.
76-86: 🛠️ Refactor suggestion
Add error handling for missing data.
The component assumes data will always be present for each field. Consider adding error handling for missing data.
-{Object.values(data).map((obj, idx) => {
+{Object.values(data).map((obj, idx) => {
+ if (!obj || typeof obj !== 'object') {
+ console.error(`Invalid data object at index ${idx}`);
+ return (
+ <td key={`${row.field}-${idx}`} className="w-80 border border-l-2 border-secondary-500 bg-secondary-100 p-2 text-center font-medium">
+ -
+ </td>
+ );
+ }
const value = obj[row.field!];
return (
<tdsrc/Components/Facility/ConsultationDetails/ConsultationNursingTab.tsx (2)
169-175: Specify explicit types instead of using any
Using explicit types improves type safety and code readability. Replace any with the appropriate type.
17-17: Verify consistent spelling of LogUpdateAnalayseTable
Ensure that the spelling of LogUpdateAnalayseTable is consistent across the codebase to prevent import errors.
Run the following script to check for inconsistent usage:
✅ Verification successful
Spelling of LogUpdateAnalayseTable is consistent across the codebase
The component name LogUpdateAnalayseTable is spelled consistently in all occurrences across the codebase, including both import statements and component usage in ConsultationNursingTab.tsx and its definition in LogUpdateAnalayseTable.tsx.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for inconsistent spelling of LogUpdateAnalayseTable.
# Expected: All occurrences should use 'LogUpdateAnalayseTable'.
rg 'LogUpdateAnal(y|ay)seTable' -g '*.tsx'
Length of output: 670
| {Object.keys(data).map((date) => ( | ||
| <> | ||
| <th | ||
| key={date} | ||
| className="w-40 border border-b-2 border-secondary-500 border-b-black p-1 text-sm font-semibold" | ||
| > | ||
| <p>{formatDate(date)}</p> | ||
| <p>{formatTime(date)}</p> | ||
| </th> | ||
| </> | ||
| ))} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add key prop to Fragment and improve accessibility.
- The Fragment in the date header mapping is missing a key prop
- The table could benefit from better accessibility
{Object.keys(data).map((date) => (
- <>
+ <React.Fragment key={date}>
<th
- key={date}
className="w-40 border border-b-2 border-secondary-500 border-b-black p-1 text-sm font-semibold"
>
<p>{formatDate(date)}</p>
<p>{formatTime(date)}</p>
</th>
- </>
+ </React.Fragment>
))}Also, add an aria-label to the table:
-<table className="border-collapse rounded-lg border bg-secondary-100">
+<table
+ className="border-collapse rounded-lg border bg-secondary-100"
+ aria-label={t('nursing_care_table_label')}
+>Committable suggestion skipped: line range outside the PR's diff.
| const { res, data } = await request(routes.dailyRoundsAnalyse, { | ||
| body: { page: currentPage, fields: NursingPlotFields }, | ||
| pathParams: { consultationId }, | ||
| }); | ||
| if (res && res.ok && data) { | ||
| setResults(data.results); | ||
| setTotalCount(data.count); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for failed API requests
Currently, there is no error handling if the API request fails. Consider adding error handling to improve user feedback.
Apply this diff:
});
+ if (!res || !res.ok) {
+ // Handle error
+ console.error('Failed to fetch daily rounds data');
+ // You may also set an error state to display a user-friendly message
+ return;
+ }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome
[error] 110-110: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
|
cosing in favor of #9079 github doesn't allow pushing changes to workflows on contributors repos |
|
I’m really sorry if I didn’t do it right. I’m still figuring things out, and I appreciate your patience. I’ll try to do better next time. Thanks for understanding. @rithviknishad I learned a lot, Thanks for the guidance 😀. |
|
@Sahil-Sinha-11 you can reopen the pr and revert changes to the workflows folder, I had to close it because github was not allowing me to push to it I'll add the changes from reviews to this pr itself |
Changes made
fixes: #8562
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
NursingPlotcomponent for enhanced display of nursing care data with pagination.LogUpdateAnalayseTablefor improved presentation of consultation data in a structured table format.Bug Fixes
Refactor
ConsultationNursingTabto integrate new components and streamline data presentation.