-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[WEB-1882] chore: optimised issue activity and updated the popover component in issue detail and peek overview #5208
Conversation
…issue detail and peek overview
WalkthroughThe recent changes introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ActivityFilterRoot
participant ActivityFilter
User->>ActivityFilterRoot: Toggle filter
ActivityFilterRoot->>ActivityFilter: Update selected filters
ActivityFilter-->>User: Display updated filters
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- web/ce/components/issues/worklog/activity/filter-root.tsx (1 hunks)
- web/ce/components/issues/worklog/activity/index.ts (1 hunks)
- web/ce/constants/issues.ts (1 hunks)
- web/core/components/issues/issue-detail/issue-activity/activity-filter.tsx (1 hunks)
- web/core/components/issues/issue-detail/issue-activity/root.tsx (2 hunks)
- web/ee/components/issues/worklog/activity/filter-root.tsx (1 hunks)
- web/ee/components/issues/worklog/activity/index.ts (1 hunks)
- web/helpers/date-time.helper.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- web/ce/components/issues/worklog/activity/index.ts
- web/ee/components/issues/worklog/activity/filter-root.tsx
Additional comments not posted (8)
web/ee/components/issues/worklog/activity/index.ts (1)
3-4
: LGTM!The new export statement enhances modularity and reusability by re-exporting all exports from
filter-root
.web/ce/constants/issues.ts (1)
21-26
: LGTM!The new type definition
TActivityFilterOption
adds clarity and type safety for activity filter options.web/ce/components/issues/worklog/activity/filter-root.tsx (1)
1-28
: LGTM!The new
ActivityFilterRoot
component is well-structured and effectively uses the newly defined typeTActivityFilterOption
. The mapping of filter options and rendering of theActivityFilter
component are clear and concise.web/core/components/issues/issue-detail/issue-activity/activity-filter.tsx (2)
1-8
: LGTM!The import statements have been updated correctly to align with the refactored component structure.
10-12
: LGTM!The type definition for
TActivityFilter
is correct and aligns with the new props structure.web/core/components/issues/issue-detail/issue-activity/root.tsx (2)
10-15
: LGTM!The import statements have been updated correctly to replace
ActivityFilter
withActivityFilterRoot
.
123-123
: LGTM!The
ActivityFilter
component has been replaced withActivityFilterRoot
, aligning with the new filtering mechanism and improving functionality.web/helpers/date-time.helper.ts (1)
326-334
: LGTM!The
convertMinutesToHoursMinutesString
function simplifies the output to focus solely on hours and minutes, reducing complexity and improving usability.
export const ActivityFilter: FC<TActivityFilter> = observer((props) => { | ||
const { selectedFilters = [], filterOptions } = props; | ||
|
||
<Transition | ||
as={Fragment} | ||
enter="transition ease-out duration-200" | ||
enterFrom="opacity-0 translate-y-1" | ||
enterTo="opacity-100 translate-y-0" | ||
leave="transition ease-in duration-150" | ||
leaveFrom="opacity-100 translate-y-0" | ||
leaveTo="opacity-0 translate-y-1" | ||
return ( | ||
<PopoverMenu | ||
buttonClassName="outline-none" | ||
button={ | ||
<Button | ||
variant="neutral-primary" | ||
size="sm" | ||
prependIcon={<ListFilter className="h-3 w-3" />} | ||
className="relative" | ||
> | ||
<span className="text-custom-text-200">Filters</span> | ||
</Button> | ||
} | ||
panelClassName="p-2 rounded-md border border-custom-border-200 bg-custom-background-100" | ||
data={filterOptions} | ||
keyExtractor={(item) => item.key} | ||
render={(item) => ( | ||
<div | ||
key={item.key} | ||
className="flex items-center gap-2 text-sm cursor-pointer px-2 p-1 transition-all hover:bg-custom-background-80 rounded-sm" | ||
onClick={item.onClick} | ||
> | ||
<div | ||
className={cn( | ||
"flex-shrink-0 w-3 h-3 flex justify-center items-center rounded-sm transition-all bg-custom-background-90", | ||
{ | ||
"bg-custom-primary text-white": item.isSelected, | ||
"bg-custom-background-80 text-custom-text-400": item.isSelected && selectedFilters.length === 1, | ||
"bg-custom-background-90": !item.isSelected, | ||
} | ||
)} | ||
> | ||
<Popover.Panel className="absolute mt-2 right-0 z-10 min-w-40"> | ||
<div className="p-2 rounded-md border border-custom-border-200 bg-custom-background-100"> | ||
{Object.keys(ACTIVITY_FILTER_TYPE_OPTIONS).map((key) => { | ||
const filterKey = key as TActivityFilters; | ||
const filter = ACTIVITY_FILTER_TYPE_OPTIONS[filterKey]; | ||
const isSelected = selectedFilters.includes(filterKey); | ||
return ( | ||
<div | ||
key={filterKey} | ||
className="flex items-center gap-2 text-sm cursor-pointer px-2 p-1 transition-all hover:bg-custom-background-80 rounded-sm" | ||
onClick={() => toggleFilter(filterKey)} | ||
> | ||
<div | ||
className={cn( | ||
"flex-shrink-0 w-3 h-3 flex justify-center items-center rounded-sm transition-all bg-custom-background-90", | ||
{ | ||
"bg-custom-primary text-white": isSelected, | ||
"bg-custom-background-80 text-custom-text-400": isSelected && selectedFilters.length === 1, | ||
"bg-custom-background-90": !isSelected, | ||
} | ||
)} | ||
> | ||
{isSelected && <Check className="h-2.5 w-2.5" />} | ||
</div> | ||
<div | ||
className={cn( | ||
"whitespace-nowrap", | ||
isSelected ? "text-custom-text-100" : "text-custom-text-200" | ||
)} | ||
> | ||
{filter.label} | ||
</div> | ||
</div> | ||
); | ||
})} | ||
</div> | ||
</Popover.Panel> | ||
</Transition> | ||
</> | ||
{item.isSelected && <Check className="h-2.5 w-2.5" />} | ||
</div> | ||
<div className={cn("whitespace-nowrap", item.isSelected ? "text-custom-text-100" : "text-custom-text-200")}> | ||
{item.label} | ||
</div> | ||
</div> | ||
)} | ||
</Popover> | ||
/> |
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.
LGTM! Consider adding tests for the new component structure.
The ActivityFilter
component has been refactored to use PopoverMenu
, improving modularity and maintainability. The rendering logic is now more streamlined.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
summary:
In this PR we have optimised the issue activity in issue detail and peek overview. And also updated the popover UI component
Changes:
Plane Issue:
[WEB-1882]
Summary by CodeRabbit
New Features
Refactor
ActivityFilter
component by integrating it with a newPopoverMenu
, improving maintainability.Documentation
Chores