-
Notifications
You must be signed in to change notification settings - Fork 558
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
Added search and status filters to questionnaires Page #10676
Conversation
WalkthroughThe pull request modifies the Changes
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionnaireList.tsx (2)
28-42
: Consider moving EmptyState to a separate component file.The EmptyState component is well-implemented but could be reused across the application. Consider moving it to a shared components directory.
-function EmptyState() { - return ( - <Card className="flex flex-col items-center justify-center p-8 text-center border-dashed"> - <div className="rounded-full bg-primary/10 p-3 mb-4"> - <CareIcon icon="l-folder-open" className="h-6 w-6 text-primary" /> - </div> - <h3 className="text-lg font-semibold mb-1"> - {t("no_questionnaires_found")} - </h3> - <p className="text-sm text-gray-500 mb-4"> - {t("adjust_questionnaire_filters")} - </p> - </Card> - ); -}Create a new file
src/components/Common/EmptyState.tsx
:interface EmptyStateProps { title: string; description: string; icon?: string; } export function EmptyState({ title, description, icon = "l-folder-open" }: EmptyStateProps) { return ( <Card className="flex flex-col items-center justify-center p-8 text-center border-dashed"> <div className="rounded-full bg-primary/10 p-3 mb-4"> <CareIcon icon={icon} className="h-6 w-6 text-primary" /> </div> <h3 className="text-lg font-semibold mb-1">{title}</h3> <p className="text-sm text-gray-500 mb-4">{description}</p> </Card> ); }
158-218
: Add keyboard navigation and sorting to the table.The table implementation could be enhanced with:
- Keyboard navigation for better accessibility
- Column sorting functionality
<tr key={questionnaire.id} onClick={() => navigate(`/questionnaire/${questionnaire.slug}`)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + navigate(`/questionnaire/${questionnaire.slug}`); + } + }} + tabIndex={0} + role="link" className="cursor-pointer hover:bg-gray-50" >Consider adding sorting functionality:
interface SortConfig { key: keyof QuestionnaireDetail; direction: 'asc' | 'desc'; } // Add sort state const [sortConfig, setSortConfig] = useState<SortConfig>({ key: 'title', direction: 'asc' }); // Add sort handler to column headers <th className="px-6 py-3 text-left text-xs font-medium uppercase tracking-wider text-gray-500 cursor-pointer" onClick={() => setSortConfig({ key: 'title', direction: sortConfig.direction === 'asc' ? 'desc' : 'asc' })} > {t("title")} {sortConfig.key === 'title' && ( <CareIcon icon={sortConfig.direction === 'asc' ? 'l-arrow-up' : 'l-arrow-down'} className="ml-1" /> )} </th>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionnaireList.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (1)
src/components/Questionnaire/QuestionnaireList.tsx (1)
5-21
: LGTM! Well-organized imports.The imports are logically grouped and follow a consistent pattern, separating utility functions, UI components, and custom components.
<div className="flex flex-wrap items-center my-2 gap-6"> | ||
<Popover> | ||
<PopoverTrigger asChild> | ||
<Button | ||
variant="outline" | ||
size="sm" | ||
className={cn( | ||
"h-8 min-w-[7.5rem] justify-start", | ||
title && "bg-primary/10 text-primary hover:bg-primary/20", | ||
)} | ||
> | ||
<CareIcon icon="l-search" className="mr-2 h-4 w-4" /> | ||
{title ? <span className="truncate">{title}</span> : t("search")} | ||
</Button> | ||
</PopoverTrigger> | ||
<PopoverContent | ||
className="w-[calc(100vw-7rem)] max-w-sm p-3" | ||
align="start" | ||
onEscapeKeyDown={(event) => event.preventDefault()} | ||
> | ||
<div className="space-y-4"> | ||
<h4 className="font-medium leading-none"> | ||
{t("search_questionnaire")} | ||
</h4> | ||
<Input | ||
id="questionnaire-search" | ||
type="text" | ||
placeholder={t("search_by_questionnaire_title")} | ||
value={title} | ||
onChange={(event) => | ||
updateQuery({ | ||
status, | ||
title: event.target.value, | ||
}) | ||
} | ||
className="cursor-pointer hover:bg-gray-50" | ||
> | ||
<td className="whitespace-nowrap px-6 py-4"> | ||
<div className="text-sm font-medium text-gray-900"> | ||
{questionnaire.title} | ||
</div> | ||
</td> | ||
<td className="px-6 py-4"> | ||
<div className="max-w-md truncate text-sm text-gray-900"> | ||
{questionnaire.description} | ||
</div> | ||
</td> | ||
<td className="px-6 py-4 whitespace-nowrap"> | ||
<Badge | ||
className={ | ||
questionnaire.status === "active" | ||
? "bg-green-100 text-green-800 hover:bg-green-200" | ||
: "" | ||
} | ||
> | ||
{questionnaire.status} | ||
</Badge> | ||
</td> | ||
<td className="px-6 py-4 whitespace-nowrap text-sm text-gray-500"> | ||
{questionnaire.slug} | ||
</td> | ||
/> | ||
</div> | ||
</PopoverContent> | ||
</Popover> | ||
<div className="items-center"> | ||
<Tabs value={status || "all"} className="w-full"> | ||
<TabsList className="bg-transparent p-0 h-8"> | ||
{statusTabs.map(({ value, label }) => ( | ||
<TabsTrigger | ||
key={value} | ||
value={value} | ||
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | ||
onClick={() => | ||
updateQuery({ | ||
status: value !== "all" ? value : undefined, | ||
title, | ||
}) | ||
} | ||
> | ||
{label} | ||
</TabsTrigger> | ||
))} | ||
</TabsList> | ||
</Tabs> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance accessibility for filter components.
While the filter implementation is good, it needs accessibility improvements:
- Add ARIA labels for screen readers
- Add keyboard navigation support
<Popover>
<PopoverTrigger asChild>
<Button
variant="outline"
size="sm"
+ aria-label={t("search_questionnaire")}
className={cn(
"h-8 min-w-[7.5rem] justify-start",
title && "bg-primary/10 text-primary hover:bg-primary/20",
)}
>
<CareIcon icon="l-search" className="mr-2 h-4 w-4" />
{title ? <span className="truncate">{title}</span> : t("search")}
</Button>
</PopoverTrigger>
<PopoverContent
className="w-[calc(100vw-7rem)] max-w-sm p-3"
align="start"
- onEscapeKeyDown={(event) => event.preventDefault()}
+ onEscapeKeyDown={(event) => {
+ updateQuery({ title: "" });
+ }}
>
📝 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.
<div className="flex flex-wrap items-center my-2 gap-6"> | |
<Popover> | |
<PopoverTrigger asChild> | |
<Button | |
variant="outline" | |
size="sm" | |
className={cn( | |
"h-8 min-w-[7.5rem] justify-start", | |
title && "bg-primary/10 text-primary hover:bg-primary/20", | |
)} | |
> | |
<CareIcon icon="l-search" className="mr-2 h-4 w-4" /> | |
{title ? <span className="truncate">{title}</span> : t("search")} | |
</Button> | |
</PopoverTrigger> | |
<PopoverContent | |
className="w-[calc(100vw-7rem)] max-w-sm p-3" | |
align="start" | |
onEscapeKeyDown={(event) => event.preventDefault()} | |
> | |
<div className="space-y-4"> | |
<h4 className="font-medium leading-none"> | |
{t("search_questionnaire")} | |
</h4> | |
<Input | |
id="questionnaire-search" | |
type="text" | |
placeholder={t("search_by_questionnaire_title")} | |
value={title} | |
onChange={(event) => | |
updateQuery({ | |
status, | |
title: event.target.value, | |
}) | |
} | |
className="cursor-pointer hover:bg-gray-50" | |
> | |
<td className="whitespace-nowrap px-6 py-4"> | |
<div className="text-sm font-medium text-gray-900"> | |
{questionnaire.title} | |
</div> | |
</td> | |
<td className="px-6 py-4"> | |
<div className="max-w-md truncate text-sm text-gray-900"> | |
{questionnaire.description} | |
</div> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap"> | |
<Badge | |
className={ | |
questionnaire.status === "active" | |
? "bg-green-100 text-green-800 hover:bg-green-200" | |
: "" | |
} | |
> | |
{questionnaire.status} | |
</Badge> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap text-sm text-gray-500"> | |
{questionnaire.slug} | |
</td> | |
/> | |
</div> | |
</PopoverContent> | |
</Popover> | |
<div className="items-center"> | |
<Tabs value={status || "all"} className="w-full"> | |
<TabsList className="bg-transparent p-0 h-8"> | |
{statusTabs.map(({ value, label }) => ( | |
<TabsTrigger | |
key={value} | |
value={value} | |
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | |
onClick={() => | |
updateQuery({ | |
status: value !== "all" ? value : undefined, | |
title, | |
}) | |
} | |
> | |
{label} | |
</TabsTrigger> | |
))} | |
</TabsList> | |
</Tabs> | |
</div> | |
</div> | |
<div className="flex flex-wrap items-center my-2 gap-6"> | |
<Popover> | |
<PopoverTrigger asChild> | |
<Button | |
variant="outline" | |
size="sm" | |
aria-label={t("search_questionnaire")} | |
className={cn( | |
"h-8 min-w-[7.5rem] justify-start", | |
title && "bg-primary/10 text-primary hover:bg-primary/20", | |
)} | |
> | |
<CareIcon icon="l-search" className="mr-2 h-4 w-4" /> | |
{title ? <span className="truncate">{title}</span> : t("search")} | |
</Button> | |
</PopoverTrigger> | |
<PopoverContent | |
className="w-[calc(100vw-7rem)] max-w-sm p-3" | |
align="start" | |
onEscapeKeyDown={(event) => { | |
updateQuery({ title: "" }); | |
}} | |
> | |
<div className="space-y-4"> | |
<h4 className="font-medium leading-none"> | |
{t("search_questionnaire")} | |
</h4> | |
<Input | |
id="questionnaire-search" | |
type="text" | |
placeholder={t("search_by_questionnaire_title")} | |
value={title} | |
onChange={(event) => | |
updateQuery({ | |
status, | |
title: event.target.value, | |
}) | |
} | |
/> | |
</div> | |
</PopoverContent> | |
</Popover> | |
<div className="items-center"> | |
<Tabs value={status || "all"} className="w-full"> | |
<TabsList className="bg-transparent p-0 h-8"> | |
{statusTabs.map(({ value, label }) => ( | |
<TabsTrigger | |
key={value} | |
value={value} | |
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | |
onClick={() => | |
updateQuery({ | |
status: value !== "all" ? value : undefined, | |
title, | |
}) | |
} | |
> | |
{label} | |
</TabsTrigger> | |
))} | |
</TabsList> | |
</Tabs> | |
</div> | |
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/components/Questionnaire/QuestionnaireList.tsx (2)
51-61
:⚠️ Potential issueAdd error handling for the query.
The query implementation is missing error handling, which could lead to a poor user experience when API requests fail.
96-134
: 🛠️ Refactor suggestionImprove search filter accessibility and behavior.
The search filter implementation needs accessibility improvements and better keyboard interaction.
Apply these changes:
<Button variant="outline" size="sm" + aria-label={t("search_questionnaire")} className={cn( "h-8 min-w-[7.5rem] justify-start", title && "bg-primary/10 text-primary hover:bg-primary/20", )} > <CareIcon icon="l-search" className="mr-2 h-4 w-4" /> {title ? <span className="truncate">{title}</span> : t("search")} </Button> ... <PopoverContent className="w-[calc(100vw-7rem)] max-w-sm p-3" align="start" - onEscapeKeyDown={(event) => event.preventDefault()} + onEscapeKeyDown={() => updateQuery({ title: "" })} >
🧹 Nitpick comments (1)
src/components/Questionnaire/QuestionnaireList.tsx (1)
28-42
: Enhance accessibility of EmptyState component.The EmptyState component should be more accessible to screen readers.
Apply this diff to improve accessibility:
function EmptyState() { return ( - <Card className="flex flex-col items-center justify-center p-8 text-center border-dashed"> + <Card className="flex flex-col items-center justify-center p-8 text-center border-dashed" role="alert" aria-live="polite"> <div className="rounded-full bg-primary/10 p-3 mb-4"> - <CareIcon icon="l-folder-open" className="h-6 w-6 text-primary" /> + <CareIcon icon="l-folder-open" className="h-6 w-6 text-primary" aria-hidden="true" /> </div> <h3 className="text-lg font-semibold mb-1"> {t("no_questionnaires_found")} </h3>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionnaireList.tsx
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
{questionnaireList.map((questionnaire: QuestionnaireDetail) => ( | ||
<tr | ||
key={questionnaire.id} | ||
onClick={() => | ||
navigate(`/questionnaire/${questionnaire.slug}`) | ||
} | ||
className="cursor-pointer hover:bg-gray-50" | ||
> | ||
<td className="whitespace-nowrap px-6 py-4"> | ||
<div className="text-sm font-medium text-gray-900"> | ||
{questionnaire.title} | ||
</div> | ||
</td> | ||
<td className="px-6 py-4"> | ||
<div className="max-w-md truncate text-sm text-gray-900"> | ||
{questionnaire.description} | ||
</div> | ||
</td> | ||
<td className="px-6 py-4 whitespace-nowrap"> | ||
<Badge | ||
className={ | ||
questionnaire.status === "active" | ||
? "bg-green-100 text-green-800 hover:bg-green-200" | ||
: "" | ||
} | ||
> | ||
{questionnaire.status} | ||
</Badge> | ||
</td> | ||
<td className="px-6 py-4 whitespace-nowrap text-sm text-gray-500"> | ||
{questionnaire.slug} | ||
</td> | ||
</tr> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add keyboard navigation to table rows.
The table rows are clickable but lack keyboard navigation support.
Apply these changes:
<tr
key={questionnaire.id}
+ role="button"
+ tabIndex={0}
onClick={() => navigate(`/questionnaire/${questionnaire.slug}`)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ navigate(`/questionnaire/${questionnaire.slug}`);
+ }
+ }}
className="cursor-pointer hover:bg-gray-50"
>
📝 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.
{questionnaireList.map((questionnaire: QuestionnaireDetail) => ( | |
<tr | |
key={questionnaire.id} | |
onClick={() => | |
navigate(`/questionnaire/${questionnaire.slug}`) | |
} | |
className="cursor-pointer hover:bg-gray-50" | |
> | |
<td className="whitespace-nowrap px-6 py-4"> | |
<div className="text-sm font-medium text-gray-900"> | |
{questionnaire.title} | |
</div> | |
</td> | |
<td className="px-6 py-4"> | |
<div className="max-w-md truncate text-sm text-gray-900"> | |
{questionnaire.description} | |
</div> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap"> | |
<Badge | |
className={ | |
questionnaire.status === "active" | |
? "bg-green-100 text-green-800 hover:bg-green-200" | |
: "" | |
} | |
> | |
{questionnaire.status} | |
</Badge> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap text-sm text-gray-500"> | |
{questionnaire.slug} | |
</td> | |
</tr> | |
{questionnaireList.map((questionnaire: QuestionnaireDetail) => ( | |
<tr | |
key={questionnaire.id} | |
role="button" | |
tabIndex={0} | |
onClick={() => navigate(`/questionnaire/${questionnaire.slug}`)} | |
onKeyDown={(e) => { | |
if (e.key === 'Enter' || e.key === ' ') { | |
e.preventDefault(); | |
navigate(`/questionnaire/${questionnaire.slug}`); | |
} | |
}} | |
className="cursor-pointer hover:bg-gray-50" | |
> | |
<td className="whitespace-nowrap px-6 py-4"> | |
<div className="text-sm font-medium text-gray-900"> | |
{questionnaire.title} | |
</div> | |
</td> | |
<td className="px-6 py-4"> | |
<div className="max-w-md truncate text-sm text-gray-900"> | |
{questionnaire.description} | |
</div> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap"> | |
<Badge | |
className={ | |
questionnaire.status === "active" | |
? "bg-green-100 text-green-800 hover:bg-green-200" | |
: "" | |
} | |
> | |
{questionnaire.status} | |
</Badge> | |
</td> | |
<td className="px-6 py-4 whitespace-nowrap text-sm text-gray-500"> | |
{questionnaire.slug} | |
</td> | |
</tr> | |
))} |
<div className="items-center"> | ||
<Tabs value={status || "all"} className="w-full"> | ||
<TabsList className="bg-transparent p-0 h-8"> | ||
{statusTabs.map(({ value, label }) => ( | ||
<TabsTrigger | ||
key={value} | ||
value={value} | ||
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | ||
onClick={() => | ||
updateQuery({ | ||
status: value !== "all" ? value : undefined, | ||
title, | ||
}) | ||
} | ||
> | ||
{label} | ||
</TabsTrigger> | ||
))} | ||
</TabsList> | ||
</Tabs> | ||
</div> | ||
</div> |
There was a problem hiding this comment.
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 to status tabs.
The status filter tabs need proper ARIA labels for screen readers.
Apply this change:
<Tabs value={status || "all"} className="w-full">
- <TabsList className="bg-transparent p-0 h-8">
+ <TabsList className="bg-transparent p-0 h-8" aria-label={t("status_filter")}>
{statusTabs.map(({ value, label }) => (
<TabsTrigger
key={value}
value={value}
+ aria-label={t("filter_by_status", { status: label })}
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary"
📝 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.
<div className="items-center"> | |
<Tabs value={status || "all"} className="w-full"> | |
<TabsList className="bg-transparent p-0 h-8"> | |
{statusTabs.map(({ value, label }) => ( | |
<TabsTrigger | |
key={value} | |
value={value} | |
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | |
onClick={() => | |
updateQuery({ | |
status: value !== "all" ? value : undefined, | |
title, | |
}) | |
} | |
> | |
{label} | |
</TabsTrigger> | |
))} | |
</TabsList> | |
</Tabs> | |
</div> | |
</div> | |
<div className="items-center"> | |
<Tabs value={status || "all"} className="w-full"> | |
<TabsList className="bg-transparent p-0 h-8" aria-label={t("status_filter")}> | |
{statusTabs.map(({ value, label }) => ( | |
<TabsTrigger | |
key={value} | |
value={value} | |
aria-label={t("filter_by_status", { status: label })} | |
className="data-[state=active]:bg-primary/10 data-[state=active]:text-primary" | |
onClick={() => | |
updateQuery({ | |
status: value !== "all" ? value : undefined, | |
title, | |
}) | |
} | |
> | |
{label} | |
</TabsTrigger> | |
))} | |
</TabsList> | |
</Tabs> | |
</div> | |
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
1967-1967
: New Search Button/Label for Questionnaires
The key"search_questionnaire"
with the value "Search Questionnaire" appears to serve as a label—for example, on a button or a header—instead of an input placeholder. Please verify that its intended usage is distinct from"search_by_questionnaire_title"
. If the context is for initiating the search rather than guiding what to search by, this distinction is appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: CodeQL-Build
- GitHub Check: lint
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
public/locale/en.json (3)
330-330
: New Localization Entry for Questionnaire Filter Adjustment
The key"adjust_questionnaire_filters"
with the value "Try adjusting your filters or create a new questionnaire" is clear, concise, and stylistically consistent with the similar"adjust_resource_filters"
entry on the following line.
1883-1883
: New Status Label Addition
The new key"retired"
with the value "Retired" is brief and fits the convention used for other status labels in the file. This will help in categorizing questionnaires or resources that are no longer active.
1949-1949
: New Search Prompt for Questionnaire Titles
The key"search_by_questionnaire_title"
provides a useful prompt for users to filter questionnaires by title. Its phrasing is clear and aligns well with the style of similar keys used elsewhere in this locale file.
@rithviknishad I have updated this PR can you just have a look |
Components were redesigned in #10678 (which includes search and tabs), so closing this PR. |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Refactor