feat: mkt digest section + misc tweaks #37
Conversation
sweetr-dev
bot
added
the
medium
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces several changes across multiple components in the application. A new component, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (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 (
|
waltergalvao
changed the title
waltergalvao
changed the title
![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
🧹 Outside diff range and nitpick comments (5)
apps/mkt/components/features-slack.tsx (4)
6-7: Remove unused importsThe following imports are not used in the component:
- FeatureIllustration
- FeatureImage01
-import FeatureIllustration from "@/public/images/home.png"; -import FeatureImage01 from "@/public/images/home.png";
22-24: Optimize heightFix function with useCallbackThe heightFix function should be memoized since it's used in both useEffect and event handlers.
+ const heightFix = useCallback(() => { + if (tabs.current?.parentElement) { + tabs.current.parentElement.style.height = `${tabs.current.clientHeight}px`; + } + }, []); useEffect(() => { heightFix(); }, [heightFix]);
41-84: Reduce code duplication in tab buttonsConsider extracting the tab button into a reusable component to improve maintainability and reduce duplication.
+ type TabButtonProps = { + isActive: boolean; + onClick: () => void; + title: string; + description: string; + icon: React.ReactNode; + }; + + const TabButton: React.FC<TabButtonProps> = ({ isActive, onClick, title, description, icon }) => ( + <button + className={`hover:scale-105 transition-all duration-500 text-left px-4 py-5 border border-dark-400 rounded ${ + !isActive ? "bg-dark-800 opacity-60 hover:opacity-100 transition" : "bg-dark-900 shadow-sm" + }`} + onClick={onClick} + > + <div className="flex items-center justify-between mb-1"> + <div className="text-xl font-inter-tight font-semibold text-zinc-200"> + {title} + </div> + <div className={isActive ? "text-green-400" : "text-white"}> + {icon} + </div> + </div> + <div className="text-sm text-zinc-400">{description}</div> + </button> + ); // Usage: <div className="grid grid-cols-2 md:grid-cols-2 gap-4 md:gap-6"> <TabButton isActive={tab === 1} onClick={(e) => { e.preventDefault(); setTab(1); }} title="Metrics Digest" description="Summary of Pull Request metrics for teams to identify bottlenecks and understand changes impact." icon={<IconChartBar stroke={1} />} /> <TabButton isActive={tab === 2} onClick={(e) => { e.preventDefault(); setTab(2); }} title="Work In Progress Digest" description="Summary of Pull Requests that are in progress to keep teams in sync." icon={<IconProgress stroke={1.5} />} /> </div>
90-136: Optimize tab content implementationTwo suggestions to improve the tab content implementation:
- Extract the transition wrapper to reduce code duplication
- Enhance image responsiveness with proper sizing strategy
+ const TabContent: React.FC<{ + show: boolean; + children: React.ReactNode; + onEnter: () => void; + }> = ({ show, children, onEnter }) => ( + <Transition + show={show} + className="w-full text-center" + enter="transition ease-in-out duration-700 transform order-first" + enterFrom="opacity-0 -translate-y-4" + enterTo="opacity-100 translate-y-0" + leave="transition ease-in-out duration-300 transform absolute" + leaveFrom="opacity-100 translate-y-0" + leaveTo="opacity-0 translate-y-4" + beforeEnter={onEnter} + unmount={false} + > + {children} + </Transition> + ); // Usage: <div className="relative flex flex-col pt-12 mx-6" ref={tabs}> <TabContent show={tab === 1} onEnter={heightFix}> <div className="inline-flex relative align-top"> <Image className="rounded-lg box-content w-full h-auto" src={DigestMetrics} quality={100} sizes="(max-width: 860px) 100vw, 860px" priority alt="Metrics Digest" /> </div> </TabContent> {/* Similar for tab 2 */} </div>apps/mkt/components/features-traits.tsx (1)
182-182: Consider optimizing the z-index valueThe z-index value of 10000 seems unnecessarily high. Consider using a lower value that still achieves the desired stacking context. This helps maintain a more manageable z-index hierarchy.
- className="h-[250px] mt-[27px] lg:h-[280px] lg:max-h-[280px] border border-dark-400 w-auto object-cover mx-auto object-left sm:object-contain sm:h-auto rounded-t-lg opacity-90 shadow-[0_0_20px_3px_rgba(255,255,255,0.25)] block z-[10000]" + className="h-[250px] mt-[27px] lg:h-[280px] lg:max-h-[280px] border border-dark-400 w-auto object-cover mx-auto object-left sm:object-contain sm:h-auto rounded-t-lg opacity-90 shadow-[0_0_20px_3px_rgba(255,255,255,0.25)] block z-[1]"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
apps/mkt/public/images/digest-metrics.pngis excluded by!**/*.pngapps/mkt/public/images/digest-wip.pngis excluded by!**/*.pngapps/mkt/public/images/git-repo.jpgis excluded by!**/*.jpg
📒 Files selected for processing (5)
apps/mkt/app/(default)/page.tsx(1 hunks)apps/mkt/components/features-automations.tsx(1 hunks)apps/mkt/components/features-overview.tsx(1 hunks)apps/mkt/components/features-slack.tsx(1 hunks)apps/mkt/components/features-traits.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/mkt/components/features-automations.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/mkt/components/features-slack.tsx
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (4)
apps/mkt/app/(default)/page.tsx (1)
8-8: LGTM: Clean integration of FeaturesSlack component
The FeaturesSlack component is properly imported and integrated into the Home page layout, maintaining consistency with the existing pattern.
Also applies to: 16-16
apps/mkt/components/features-overview.tsx (1)
149-149: LGTM: Text update improves inclusivity
The text change from "For managers & team leads" to "For managers & their teams" makes the messaging more inclusive.
apps/mkt/components/features-traits.tsx (2)
5-5: LGTM: Import changes are consistent
The new imports for GitRepo and IconPlugConnected are properly organized and follow the established pattern.
Also applies to: 14-14
267-275: LGTM: Feature updates are well-structured
The changes to "More Integrations" and "1:1s" features are clear, concise, and maintain consistency with the existing structure. The icon choices appropriately represent their respective features.
| const heightFix = () => { | ||
| if (tabs.current && tabs.current.parentElement) | ||
| tabs.current.parentElement.style.height = `${tabs.current.clientHeight}px`; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use optional chaining for safer property access
The current implementation could be safer using optional chaining.
- if (tabs.current && tabs.current.parentElement)
- tabs.current.parentElement.style.height = `${tabs.current.clientHeight}px`;
+ if (tabs.current?.parentElement) {
+ tabs.current.parentElement.style.height = `${tabs.current.clientHeight}px`;
+ }Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
Summary by CodeRabbit
Release Notes
New Features
FeaturesSlackcomponent, providing a tabbed interface for Slack-related digests.Improvements
FeaturesAutomationscomponent to better target developers.FeaturesOverviewcomponent for inclusivity.FeaturesTraitscomponent with new images and updated feature descriptions for clarity.These changes aim to improve user experience and communication within the application.