feat: activity tab on schema explorer [ENG-1647]#7162
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
40f657d to
471cb51
Compare
471cb51 to
047cce0
Compare
047cce0 to
9d243cb
Compare
9d243cb to
dc31bbc
Compare
2899a1b to
1764fd9
Compare
1ef20e5 to
f507531
Compare
Greptile SummaryRefactored action center to support tab-based navigation and added monitor-scoped activity views. Key changes:
Issues:
Confidence Score: 4/5
Important Files Changed
|
|
|
||
| return ( | ||
| <Flex className="h-[calc(100%-48px)] overflow-hidden" gap="middle" vertical> | ||
| <Flex className="justify-between "> |
There was a problem hiding this comment.
style: update onChange to use updateSearch from useSearch hook
If switching to useSearch hook, update this to use updateSearch instead of setSearchQuery
| <Flex className="justify-between "> | |
| <DebouncedSearchInput value={searchQuery} onChange={updateSearch} /> |
Context Used: Rule from dashboard - Use the existing useSearch hook for search functionality to maintain URL state synchronization ins... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| hidden={!!filters?.monitorId} | ||
| value={filters?.monitorId ?? searchQuery ?? ""} |
There was a problem hiding this comment.
style: search input hidden but still shows monitorId as value when filtered
When filters?.monitorId is provided, the search input is hidden but still displays the monitorId as its value. Consider passing undefined or empty string when hidden for consistency
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
This is intentional for the moment
There was a problem hiding this comment.
I'm also not sure why we need to show a hidden input with a value?
There was a problem hiding this comment.
@lucanovera It's a temporary hack for how the screen works to filter by monitor.
We don't want to show the input because it shouldn't be changed by the user.
Disabling it would look broken + it's not showing any useful information.
| flags: { webMonitor: webMonitorEnabled, heliosV2: heliosV2Enabled }, | ||
| } = useFeatures(); | ||
| const { paginationProps, pageIndex, pageSize, resetPagination } = | ||
| useAntPagination(); |
There was a problem hiding this comment.
style: use useSearch hook instead of local state for search
This component uses local useState for search, but the existing useSearch hook should be used to maintain URL state synchronization (as done in other components like InProgressMonitorTasksList)
| useAntPagination(); | |
| const { searchQuery, updateSearch } = useSearch(); |
Context Used: Rule from dashboard - Use the existing useSearch hook for search functionality to maintain URL state synchronization ins... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Wrong line. Also was existing code. Trying to reduce the amount of changes in a single pr
lucanovera
left a comment
There was a problem hiding this comment.
The new menu / page routing code looks good. I see the Activity tab but it's got no results on it. Can you include instructions for how to see results in there or a link to the preview where it has results?
| defaultKey?: Exclude<keyof R, symbol | number>; | ||
| } | ||
|
|
||
| const useMenuNavigation = < |
There was a problem hiding this comment.
Thanks for implementing this as a generic we can reuse. That will be useful when refactoring old tabs with hashes for pages and menus.
| hidden={!!filters?.monitorId} | ||
| value={filters?.monitorId ?? searchQuery ?? ""} |
There was a problem hiding this comment.
I'm also not sure why we need to show a hidden input with a value?
I've updated the instructions. You just need to adjust your filters to show all the different types of events. It defaults to hide common activity. |
fix: title for tests wip: more refactoring refactor: page structure fix: minor style issues refactor: page based wip: still standardizing wip: further refactors refactor: filter related changes linting and other fixes fix: type issue fix: broken imports chore: update changelog fix: more imports
f507531 to
f67271d
Compare
lucanovera
left a comment
There was a problem hiding this comment.
UI looks good and behaves correctly, tested different filters, search, etc. Approved!
Ticket ENG-1647
Description Of Changes
Adds the
Activitytab to monitor screens so that users can view activity specific to a monitor they are working on.Code Changes
Action-Centerto contain a shared layout.Steps to Confirm
Activitytab is visibleActivitytab only displays values specific to the monitor being viewed by changing the filter to include all activity types.ActivitytabPre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works