Fix : Implement Multi-Selection for Organizations and Tags #10696
Conversation
WalkthroughThe pull request updates the organization selection UI across questionnaire components. It replaces the command-based interface with dropdown and popover components, removes obsolete state variables for search queries, and updates query keys and event handling. Localization is applied consistently to all user-facing text. Additionally, import statements and icon usage have been adjusted in the editor component, streamlining the overall selection process without altering any exported or public entities. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (2)
176-226: Enhance accessibility of the dropdown implementation.The dropdown implementation successfully replaces the search bar while maintaining search functionality. However, let's improve accessibility:
<Input placeholder={t("search_tags")} value={searchQuery} onChange={(e) => setSearchQuery(e.target.value)} className="mb-2" + aria-label={t("search_tags")} /> <div className="max-h-60 overflow-y-auto"> {isLoading ? ( <div className="flex items-center justify-center py-6"> - <Loader2 className="h-6 w-6 animate-spin" /> + <Loader2 className="h-6 w-6 animate-spin" aria-busy="true" /> </div> ) : ( availableTags?.results .filter( (tag) => !selectedSlugs.includes(tag.slug) && tag.name .toLowerCase() .includes(searchQuery.toLowerCase()), ) .map((tag) => ( - <div + <button key={tag.slug} className="flex items-center gap-2 p-2 hover:bg-accent cursor-pointer" onClick={() => handleToggleTag(tag.slug)} + type="button" + role="option" > <Hash className="h-4 w-4" /> {tag.name} - </div> + </button> )) )} </div>
110-120: Enhance tag creation validation.The current validation only checks for empty strings. Consider adding more comprehensive validation:
const handleCreateTag = () => { - if (!newTagName.trim() || !newTagSlug.trim()) { + const trimmedName = newTagName.trim(); + const trimmedSlug = newTagSlug.trim(); + + if (!trimmedName || !trimmedSlug) { toast.error("Name and slug are required"); return; } + + if (trimmedName.length > 50 || trimmedSlug.length > 50) { + toast.error(t("tag_name_too_long")); + return; + } + + if (!/^[a-z0-9-]+$/.test(trimmedSlug)) { + toast.error(t("tag_slug_invalid_characters")); + return; + } createTag({ - name: newTagName.trim(), - slug: newTagSlug.trim(), + name: trimmedName, + slug: trimmedSlug, }); };src/components/Questionnaire/CloneQuestionnaireSheet.tsx (5)
48-57: Document the purpose of the "role" org_type filter.Consider adding a comment explaining why the organization query is filtered by
org_type: "role". This will help future maintainers understand the business logic behind this filter.queryFn: query(organizationApi.list, { queryParams: { + // Filter organizations that can be assigned roles in questionnaires org_type: "role", }, }),
68-72: Internationalize error messages.For consistency with the rest of the application, consider using i18n for error messages.
- setError("This slug is already in use. Please choose a different one."); + setError(t("This slug is already in use. Please choose a different one.")); - setError("Failed to clone questionnaire. Please try again."); + setError(t("Failed to clone questionnaire. Please try again."));
77-79: Internationalize the slug validation message.For consistency, use i18n for the slug validation error message.
- setError("Slug is required"); + setError(t("Slug is required"));
195-195: Consider using responsive max-height for the dropdown menu.The fixed
max-h-[300px]might not be optimal for all screen sizes. Consider using responsive classes or CSS variables for better adaptability.-<DropdownMenuContent className="w-full min-w-[200px] max-h-[300px] overflow-auto"> +<DropdownMenuContent className="w-full min-w-[200px] max-h-[40vh] overflow-auto">
221-225: Consider truncating long organization descriptions.Long descriptions might cause layout issues. Consider truncating them with an ellipsis.
- <span className="text-xs text-gray-500"> + <span className="text-xs text-gray-500 truncate max-w-[200px]"> - {org.description} </span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx(6 hunks)src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx(4 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Test
- GitHub Check: lint
- GitHub Check: CodeQL-Build
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (1)
2-2: LGTM! Clean setup for the dropdown implementation.The import changes and state management setup are well-structured for the new dropdown-based tag selection interface.
Also applies to: 18-23, 56-56
src/components/Questionnaire/CloneQuestionnaireSheet.tsx (1)
1-36: LGTM! Imports and type definitions are well-organized.The imports align with the PR objectives, including the necessary components for dropdown menu implementation and internationalization support.
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
438-484: Well-implemented dropdown replacement with improved UX.The new dropdown implementation includes several improvements:
- Proper loading state handling
- Prevention of duplicate organization selection
- Enhanced organization display with descriptions
- Internationalized UI text
101-105:❓ Verification inconclusive
Verify the impact of query parameter changes.
The organization query has been modified to use
idinstead of a search query. This change might affect the search functionality for organizations.Run the following script to check the usage of organization queries across the codebase:
🏁 Script executed:
#!/bin/bash # Description: Check for organization query implementations # Test: Search for organization query implementations to ensure consistency # Search for organization query keys rg -A 5 'queryKey.*organizations' # Search for organization API calls rg -A 5 'organizationApi.list'Length of output: 10030
Review Query Parameter Changes in QuestionnaireEditor
The
QuestionnaireEditornow uses:queryKey: ["organizations", id], queryFn: query(organizationApi.list, { queryParams: { org_type: "role", name: id || undefined, }, });This implementation differs from other components (e.g., using
searchQueryinManageQuestionnaireOrganizationsSheet). Please verify that usingidas the search parameter is intentional and aligns with the desired filtering behavior for organizations across the codebase.
- Confirm that
idcorrectly represents the intended search/filter value.- Ensure that replacing the dynamic search query with
idwon’t inadvertently limit functionality.- Review related components to guarantee consistent use of query parameters.
| <ChevronDown className="mr-2 h-4 w-4" /> | ||
| Move Up |
There was a problem hiding this comment.
Fix icon-action mismatch in move functionality.
The ChevronDown icon is being used for the "Move Up" action, which could be confusing for users. Consider using an upward-pointing chevron for upward movement.
Apply this diff to fix the icon-action mismatch:
- <ChevronDown className="mr-2 h-4 w-4" />
+ <ChevronUp className="mr-2 h-4 w-4" />Don't forget to add the ChevronUp import at the top of the file:
-import { Building, ChevronDown, Loader2, X } from "lucide-react";
+import { Building, ChevronDown, ChevronUp, Loader2, X } from "lucide-react";📝 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.
| <ChevronDown className="mr-2 h-4 w-4" /> | |
| Move Up | |
| // At the top of the file, update the import: | |
| import { Building, ChevronDown, ChevronUp, Loader2, X } from "lucide-react"; | |
| // ... | |
| // Later in the file where the "Move Up" action is rendered: | |
| <ChevronUp className="mr-2 h-4 w-4" /> | |
| Move Up |
|
@rithviknishad @Jacobjeevan Used Command with Popover instead of Select. Can we re-open this PR ? |
v12.mp4 |
Jacobjeevan
added
waiting for related PR
|
Wait for #10678 to be merged |
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (1)
64-73:⚠️ Potential issueFix syntax error in query configuration.
There's a syntax error in the query configuration that's causing build failures.
Apply this diff to fix the syntax:
const { data: availableTags, isLoading } = useQuery({ queryKey: ["questionnaire_tags", searchQuery], -: query(questionnaireApi.tags.list, { - queryFn: query.debounced(questionnaireApi.tags.list, { - queryParams: searchQuery !== "" ? { name: searchQuery } : undefined, }), enabled: open, });🧰 Tools
🪛 Biome (1.9.4)
[error] 65-66: Expected a property, a shorthand property, a getter, a setter, or a method but instead found ':'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
[error] 73-73: expected
,but instead found;Remove ;
(parse)
🪛 GitHub Check: cypress-run (1)
[failure] 66-66:
Property assignment expected.🪛 GitHub Actions: Lint Code Base
[error] 66-66: Parsing error: Property assignment expected
🪛 GitHub Actions: Cypress Tests
[error] 66-66: error TS1136: Property assignment expected.
🧹 Nitpick comments (5)
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
639-643: Consider consolidating related state variables.The component uses multiple separate state variables for managing selections. Consider consolidating these into a single state object for better maintainability.
- const [selectedOrgIds, setSelectedOrgIds] = useState<string[]>([]); - const [selectedTagIds, setSelectedTagIds] = useState<string[]>([]); - const [orgSearchQuery, setOrgSearchQuery] = useState(""); - const [tagSearchQuery, setTagSearchQuery] = useState(""); + const [selectionState, setSelectionState] = useState({ + organizations: { + selectedIds: [] as string[], + searchQuery: "", + }, + tags: { + selectedIds: [] as string[], + searchQuery: "", + }, + });
696-699: Enhance error handling with specific error messages.The error handling in mutation functions only shows generic error messages without utilizing the error object. Consider providing more specific error messages based on the error type.
- onError: (_error) => { + onError: (error: Error) => { + const errorMessage = error.message || "Failed to create questionnaire"; toast.error("Failed to create questionnaire"); + console.error("Questionnaire creation error:", error); }, - onError: (_error) => { + onError: (error: Error) => { + const errorMessage = error.message || "Failed to update questionnaire"; toast.error("Failed to update questionnaire"); + console.error("Questionnaire update error:", error); },Also applies to: 709-712
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (1)
191-240: Enhance dropdown accessibility.The dropdown implementation is good, but could benefit from some accessibility improvements:
Apply these enhancements:
<Popover modal={true}> <PopoverTrigger asChild> <Button variant="outline" className="w-full justify-start text-left font-normal" + aria-label={t("search_tags")} + aria-expanded={open} > <Hash className="mr-2 h-4 w-4" /> <span>{t("search_tags")}</span> + <ChevronDown className="ml-auto h-4 w-4 opacity-50" /> </Button> </PopoverTrigger> <PopoverContent className="p-0 w-[var(--radix-popover-trigger-width)]" align="start" + side="bottom" + sideOffset={4} >src/components/Questionnaire/CloneQuestionnaireSheet.tsx (2)
55-64: Consider adding pagination to organization query.While the simplified query works, consider implementing pagination to handle large sets of organizations efficiently. This would improve performance and user experience when dealing with many organizations.
const { data: availableOrganizations, isLoading: isLoadingOrganizations } = useQuery({ queryKey: ["organizations"], queryFn: query(organizationApi.list, { queryParams: { org_type: "role", + limit: 10, + offset: 0, }, }), enabled: open, });
237-266: Consider adding a confirmation dialog for cloning.While the implementation is good, consider adding a confirmation dialog before cloning to prevent accidental operations, especially since this creates a new questionnaire.
const handleClone = () => { if (!newSlug.trim()) { setError("Slug is required"); return; } + if (!window.confirm(t("Are you sure you want to clone this questionnaire?"))) { + return; + } const clonedQuestionnaire = { ...questionnaire, slug: newSlug.trim(), id: undefined, status: "draft" as const, title: `${questionnaire.title} (Clone)`, organizations: selectedIds, }; cloneQuestionnaire(clonedQuestionnaire); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx(7 hunks)src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx(4 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(8 hunks)
🧰 Additional context used
🪛 ESLint
src/components/Questionnaire/QuestionnaireEditor.tsx
[error] 3-4: Delete ⏎
(prettier/prettier)
[error] 13-14: Delete ⏎
(prettier/prettier)
[error] 52-54: Replace ⏎import·{·RadioGroup,·RadioGroupItem·}·from·"@/components/ui/radio-group";⏎ with import·{·RadioGroup,·RadioGroupItem·}·from·"@/components/ui/radio-group";
(prettier/prettier)
[error] 658-659: Delete ⏎
(prettier/prettier)
[error] 950-951: Delete ⏎
(prettier/prettier)
🪛 GitHub Check: lint
src/components/Questionnaire/QuestionnaireEditor.tsx
[failure] 3-3:
Delete ⏎
[failure] 52-52:
Replace ⏎import·{·RadioGroup,·RadioGroupItem·}·from·"@/components/ui/radio-group";⏎ with import·{·RadioGroup,·RadioGroupItem·}·from·"@/components/ui/radio-group";
[failure] 658-658:
Delete ⏎
🪛 GitHub Actions: Lint Code Base
src/components/Questionnaire/QuestionnaireEditor.tsx
[error] 3-3: Delete ⏎
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx
[error] 66-66: Parsing error: Property assignment expected
🪛 Biome (1.9.4)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx
[error] 65-66: Expected a property, a shorthand property, a getter, a setter, or a method but instead found ':'.
Expected a property, a shorthand property, a getter, a setter, or a method here.
(parse)
🪛 GitHub Check: cypress-run (1)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx
[failure] 66-66:
Property assignment expected.
🪛 GitHub Actions: Cypress Tests
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx
[error] 66-66: error TS1136: Property assignment expected.
🔇 Additional comments (5)
src/components/Questionnaire/QuestionnaireEditor.tsx (1)
1270-1271: Fix icon-action mismatch in move functionality.The ChevronDown icon is being used for both "Move Up" and "Move Down" actions, which is confusing. The icons should match their respective movement directions.
Apply this diff to fix the icon-action mismatch:
- <ChevronDown className="mr-2 h-4 w-4" /> + <ChevronUp className="mr-2 h-4 w-4" /> Move UpDon't forget to add the ChevronUp import at the top of the file if not already present.
Also applies to: 1281-1282
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (1)
103-141: LGTM! Well-structured tag management logic.The implementation of effect hooks and handlers for tag selection and state management is clean and efficient.
src/components/Questionnaire/CloneQuestionnaireSheet.tsx (3)
2-23: LGTM! Import changes align with requirements.The changes appropriately:
- Add i18next for internationalization
- Import Popover components for dropdown functionality
- Include necessary icons for the UI
66-106: LGTM! Well-implemented mutation and handlers.The code demonstrates good practices:
- Proper error handling with user feedback
- Clean organization selection state management
194-232: LGTM! Well-implemented dropdown with search functionality.The dropdown implementation:
- Provides good user experience with search capability
- Handles loading and empty states appropriately
- Uses internationalization consistently
| @@ -1,5 +1,5 @@ | |||
| import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; | |||
| import { Check, Hash, Loader2, Plus, X } from "lucide-react"; | |||
| import { ChevronDown, Hash, Loader2, Plus, X } from "lucide-react"; | |||
There was a problem hiding this comment.
Fix icon imports.
The code has mismatched icon imports:
ChevronDownis imported but not usedCheckis used but not imported
Apply this diff to fix the imports:
-import { ChevronDown, Hash, Loader2, Plus, X } from "lucide-react";
+import { Check, Hash, Loader2, Plus, X } from "lucide-react";Also applies to: 230-230
| <div className="space-y-4"> | ||
| {/* Selected Organizations Display */} | ||
| <div className="flex flex-wrap gap-2"> | ||
| {selectedIds.length > 0 ? ( | ||
| availableOrganizations?.results | ||
| .filter((org) => selectedIds.includes(org.id)) | ||
| .map((org) => ( | ||
| <Badge | ||
| key={org.id} | ||
| value={org.id} | ||
| onSelect={() => handleToggleOrganization(org.id)} | ||
| variant="secondary" | ||
| className="flex items-center gap-1" | ||
| > | ||
| <div className="flex flex-1 items-center gap-2"> | ||
| <Building className="h-4 w-4" /> | ||
| <span>{org.name}</span> | ||
| {org.description && ( | ||
| <span className="text-xs text-gray-500"> | ||
| - {org.description} | ||
| </span> | ||
| )} | ||
| </div> | ||
| {selectedIds.includes(org.id) && ( | ||
| <Check className="h-4 w-4" /> | ||
| )} | ||
| </CommandItem> | ||
| {org.name} | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="h-4 w-4 p-0 hover:bg-transparent" | ||
| onClick={() => handleToggleOrganization(org.id)} | ||
| > | ||
| <X className="h-3 w-3" /> | ||
| </Button> | ||
| </Badge> | ||
| )) | ||
| )} | ||
| </CommandGroup> | ||
| </CommandList> | ||
| </Command> | ||
| ) : ( | ||
| <p className="text-sm text-gray-500"> | ||
| {t("No organizations selected")} | ||
| </p> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove duplicated organization display code.
The organization display code is duplicated. Consider extracting it into a reusable component.
+ const SelectedOrganizations = ({
+ selectedIds,
+ organizations,
+ onRemove
+ }: {
+ selectedIds: string[],
+ organizations: typeof availableOrganizations.results,
+ onRemove: (id: string) => void
+ }) => (
+ <div className="flex flex-wrap gap-2">
+ {selectedIds.length > 0 ? (
+ organizations
+ ?.filter((org) => selectedIds.includes(org.id))
+ .map((org) => (
+ <Badge
+ key={org.id}
+ variant="secondary"
+ className="flex items-center gap-1"
+ >
+ {org.name}
+ <Button
+ variant="ghost"
+ size="icon"
+ className="h-4 w-4 p-0 hover:bg-transparent"
+ onClick={() => onRemove(org.id)}
+ >
+ <X className="h-3 w-3" />
+ </Button>
+ </Badge>
+ ))
+ ) : (
+ <p className="text-sm text-gray-500">
+ {t("No organizations selected")}
+ </p>
+ )}
+ </div>
+ );
{/* Organization Selector */}
<div className="space-y-4">
<h3 className="text-sm font-medium">Add Organizations</h3>
<div className="space-y-4">
- {/* Selected Organizations Display */}
- <div className="flex flex-wrap gap-2">
- {selectedIds.length > 0 ? (
- availableOrganizations?.results
- .filter((org) => selectedIds.includes(org.id))
- .map((org) => (
- <Badge
- key={org.id}
- variant="secondary"
- className="flex items-center gap-1"
- >
- {org.name}
- <Button
- variant="ghost"
- size="icon"
- className="h-4 w-4 p-0 hover:bg-transparent"
- onClick={() => handleToggleOrganization(org.id)}
- >
- <X className="h-3 w-3" />
- </Button>
- </Badge>
- ))
- ) : (
- <p className="text-sm text-gray-500">
- {t("No organizations selected")}
- </p>
- )}
- </div>
+ <SelectedOrganizations
+ selectedIds={selectedIds}
+ organizations={availableOrganizations?.results}
+ onRemove={handleToggleOrganization}
+ />Also applies to: 165-190
rithviknishad
added
changes required
and removed
waiting for related PR
Jeffrin2005
force-pushed
the
drop-down-selection
branch
from
dbdee49 to
683d0c7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx (1)
163-191: 🛠️ Refactor suggestionRemove duplicated organization display code.
The organization display code is duplicated. Consider extracting it into a reusable component.
Apply this diff to extract the duplicated code into a reusable component:
+ const SelectedOrganizations = ({ + selectedIds, + organizations, + onRemove + }: { + selectedIds: string[], + organizations: typeof availableOrganizations.results, + onRemove: (id: string) => void + }) => ( + <div className="flex flex-wrap gap-2"> + {selectedIds.length > 0 ? ( + organizations + ?.filter((org) => selectedIds.includes(org.id)) + .map((org) => ( + <Badge + key={org.id} + variant="secondary" + className="flex items-center gap-1" + > + {org.name} + <Button + variant="ghost" + size="icon" + className="h-4 w-4 p-0 hover:bg-transparent" + onClick={() => onRemove(org.id)} + > + <X className="h-3 w-3" /> + </Button> + </Badge> + )) + ) : ( + <p className="text-sm text-gray-500"> + {t("No organizations selected")} + </p> + )} + </div> + ); {/* Organization Selector */} <div className="space-y-4"> <h3 className="text-sm font-medium">Add Organizations</h3> <div className="space-y-4"> - {/* Selected Organizations Display */} - <div className="flex flex-wrap gap-2"> - {selectedIds.length > 0 ? ( - availableOrganizations?.results - .filter((org) => selectedIds.includes(org.id)) - .map((org) => ( - <Badge - key={org.id} - variant="secondary" - className="flex items-center gap-1" - > - {org.name} - <Button - variant="ghost" - size="icon" - className="h-4 w-4 p-0 hover:bg-transparent" - onClick={() => handleToggleOrganization(org.id)} - > - <X className="h-3 w-3" /> - </Button> - </Badge> - )) - ) : ( - <p className="text-sm text-gray-500"> - {t("No organizations selected")} - </p> - )} - </div> + <SelectedOrganizations + selectedIds={selectedIds} + organizations={availableOrganizations?.results} + onRemove={handleToggleOrganization} + />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx(7 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: CodeQL-Build
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: lint
🔇 Additional comments (5)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx (3)
2-3: LGTM!The new imports for Popover components and icons are correctly added and align with the UI changes.
Also applies to: 19-23
193-233: LGTM! The organization selection UI is well-implemented.The changes improve the user experience by:
- Using an intuitive dropdown interface.
- Adding proper loading and empty states.
- Maintaining consistent styling.
154-154: LGTM! Text strings are properly localized.All user-facing text strings are consistently wrapped with the t() function for internationalization.
Also applies to: 188-188, 197-197, 204-204, 208-208, 248-248, 259-259, 262-262
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
12-12: LGTM!The new icon imports are correctly added and align with the UI changes.
303-305: LGTM! The organization selection UI is well-implemented.The changes improve the user experience by using consistent styling, proper spacing, and alignment.
| <p className="text-sm text-gray-500"> | ||
| {t("no_organizations_selected")} | ||
| <p className="text-xs text-gray-500 text-center"> | ||
| {t("No organizations found")} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix inconsistent empty state text.
The empty state text is inconsistent:
- For organizations: "No organizations found"
- For tags: "no_tags_selected"
Use consistent text for both cases.
Apply this diff to fix the inconsistency:
- {t("no_tags_selected")}
+ {t("No tags found")}Also applies to: 412-412
v12.mp4 |
|
@Jeffrin2005 do a self review With the recent changes to UI, lot of changes in this PR is unnecessary (update the description in PR as well).
|
Jeffrin2005
force-pushed
the
drop-down-selection
branch
from
6d0451c to
677a436
Compare
Jeffrin2005
changed the title
Jeffrin2005
changed the title
|
@Jacobjeevan we only need to display the selected tags ? Is there anything to remove ? |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit