-
Notifications
You must be signed in to change notification settings - Fork 51
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
[Fix bug] Header scroll and card items in the table view | All teams page #3400
[Fix bug] Header scroll and card items in the table view | All teams page #3400
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
apps/web/app/[locale]/all-teams/component.tsxOops! Something went wrong! :( ESLint: 8.46.0 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/apps/web/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. WalkthroughThe pull request introduces structural changes to the Changes
Assessment against linked issues
Possibly related PRs
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 (
|
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
🧹 Outside diff range and nitpick comments (2)
apps/web/app/[locale]/all-teams/component.tsx (1)
57-59
: Consider adding a loading stateThe container renders teams immediately without a loading state, which might cause layout shifts during data fetching.
Consider adding a loading skeleton:
<Container fullWidth={fullWidth} className="mx-auto mt-5"> + {filteredTeams ? ( <AllTeamsMembers teams={filteredTeams} view={view} /> + ) : ( + <TeamsSkeleton /> + )} </Container>apps/web/lib/features/all-teams/users-teams-card/user-card.tsx (1)
Line range hint
71-74
: Fix hidden menu accessibilityThe menu is hidden using
hidden
class which makes it inaccessible to screen readers.-<div className="absolute hidden right-2"> +<div className="absolute invisible right-2 sm:visible">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/web/app/[locale]/all-teams/component.tsx
(2 hunks)apps/web/lib/features/all-teams/users-teams-card/user-card.tsx
(1 hunks)
🔇 Additional comments (3)
apps/web/app/[locale]/all-teams/component.tsx (2)
41-51
: LGTM: Header structure improvements address scrolling issues
The new header structure using mainHeaderSlot
with proper flex layout and spacing addresses the header index scrolling issues mentioned in bug #3393. The layout ensures proper alignment of breadcrumb and controls.
53-53
: Verify the view-dependent header rendering
The conditional rendering of TeamMemberHeader
only in CARDS view might affect the header's behavior during view switches.
✅ Verification successful
Conditional header rendering is correctly implemented
Looking at the codebase evidence:
TeamMemberHeader
component inlib/features/team-member-header.tsx
internally handles all view types (CARDS, TABLE, BLOCKS) and renders the appropriate header component based on the view prop.- The component is used consistently across the application, with proper view prop passing.
- In
all-teams/component.tsx
, the conditional rendering for CARDS view is intentional and aligns with the application's design, as this page has different layout requirements compared to other usages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other view-dependent header renderings
ast-grep --pattern 'TeamMemberHeader view={$_}'
Length of output: 49
Script:
#!/bin/bash
# Let's first find all files containing TeamMemberHeader
rg "TeamMemberHeader" -l
# Then look for its usage patterns
rg "TeamMemberHeader" -A 2 -B 2
# Also search for IssuesView enum usage
rg "IssuesView\." -A 2 -B 2
Length of output: 9629
apps/web/lib/features/all-teams/users-teams-card/user-card.tsx (1)
37-44
: LGTM: Improved card layout structure
The flex layout with justify-between
and proper cursor styling improves the card's visual structure. The grid icon positioning is now correctly handled.
Description
Fixes #3393
Type of Change
Checklist
Previous screenshots
Loom
Current screenshots
Loom
Summary by CodeRabbit
New Features
Bug Fixes
Style