[WEB-4733] dev: propel toolbar component #7742
Conversation
anmolsinghbhatia
requested review from
prateekshourya29,
sriramveeraghanta and
vamsikrishnamathala
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a new Toolbar component suite (compound API with Group/Item/Separator/SubmitButton), re-exports it and its types, includes it in the build and package exports as "./toolbar", and adds Storybook stories demonstrating usages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as Consumer App
participant Toolbar as Toolbar (compound)
participant Tooltip as Tooltip
User->>App: interacts with UI
App->>Toolbar: renders <Toolbar> with Groups/Items/SubmitButton
alt Hover or focus on Item with tooltip
Toolbar->>Tooltip: request show (text + shortcut)
Tooltip-->>User: display tooltip
end
User->>Toolbar: click Item or SubmitButton
Toolbar-->>App: invoke onClick / onSubmit (may set loading)
App-->>Toolbar: update props (e.g., loading=false)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Pull Request Linked with Plane Work Items
Comment Automatically Generated by Plane |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new toolbar component to the propel UI library. The toolbar component provides a flexible interface for grouping action buttons with support for tooltips, keyboard shortcuts, and active states.
- Adds a comprehensive toolbar component with subcomponents for groups, items, separators, and submit buttons
- Includes Storybook stories demonstrating various toolbar configurations including text formatting and comment scenarios
- Updates package configuration to export the new toolbar module
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/propel/tsdown.config.ts | Adds toolbar entry point to build configuration |
| packages/propel/src/toolbar/toolbar.tsx | Main toolbar component implementation with all subcomponents |
| packages/propel/src/toolbar/toolbar.stories.tsx | Storybook stories showcasing toolbar usage patterns |
| packages/propel/src/toolbar/index.ts | Module exports for toolbar component and types |
| packages/propel/package.json | Adds toolbar export to package entry points |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/propel/package.json (1)
35-35: Optional: expose types in subpath export for better TS resolution.TypeScript does not always infer .d.ts from string exports. Consider adding a
typescondition (you may roll this out to other entries later).- "./toolbar": "./dist/toolbar/index.js", + "./toolbar": { + "types": "./dist/toolbar/index.d.ts", + "default": "./dist/toolbar/index.js" + },packages/propel/src/toolbar/toolbar.tsx (3)
37-45: Prefer auto overflow to avoid always-visible horizontal scrollbar.Switching to
overflow-x-autoshows the scrollbar only when needed.- className={cn("flex h-9 w-full items-stretch gap-1.5 bg-custom-background-90 overflow-x-scroll", className)} + className={cn("flex h-9 w-full items-stretch gap-1.5 bg-custom-background-90 overflow-x-auto", className)}
17-24: Decouple icon type from lucide-react for flexibility.Accept any SVG React component to avoid locking consumers to
lucide-react.-import { LucideIcon } from "lucide-react"; +import type { SVGProps, ComponentType } from "react"; ... -export interface ToolbarItemProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { - icon: LucideIcon; +export interface ToolbarItemProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { + icon: ComponentType<SVGProps<SVGSVGElement>>;
123-143: SubmitButton: default to submit, expose busy state, and align right robustly.Make it a proper submit control by default, announce loading to AT, and push it to the far right within the flex container.
- ({ loading = false, variant = "primary", className, children, disabled, ...props }, ref) => ( - <div className="sticky right-1"> + ({ loading = false, variant = "primary", className, children, disabled, ...props }, ref) => ( + <div className="sticky right-1 ml-auto"> <button ref={ref} className={cn( "inline-flex items-center justify-center gap-2 rounded-md px-2.5 py-1.5 text-xs font-medium transition-colors duration-200", "focus:outline-none focus:ring-2 focus:ring-custom-primary-100/20 focus:ring-offset-2", "disabled:opacity-50 disabled:pointer-events-none", buttonVariants[variant], className )} - disabled={disabled || loading} + type={props.type ?? "submit"} + aria-busy={loading} + disabled={disabled || loading} {...props} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/propel/package.json(1 hunks)packages/propel/src/toolbar/index.ts(1 hunks)packages/propel/src/toolbar/toolbar.stories.tsx(1 hunks)packages/propel/src/toolbar/toolbar.tsx(1 hunks)packages/propel/tsdown.config.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/propel/src/toolbar/toolbar.tsx (1)
packages/propel/src/toolbar/index.ts (6)
ToolbarProps(3-3)ToolbarGroupProps(4-4)ToolbarItemProps(5-5)ToolbarSeparatorProps(6-6)ToolbarSubmitButtonProps(7-7)Toolbar(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
packages/propel/tsdown.config.ts (1)
20-20: LGTM: added toolbar entry to the build.This ensures Toolbar is emitted (JS + DTS). No action needed.
packages/propel/src/toolbar/index.ts (1)
1-8: LGTM: clean public surface re-exports.Type-only exports keep bundles lean. No changes needed.
packages/propel/src/toolbar/toolbar.stories.tsx (1)
34-67: Stories look representative; nice coverage of states.Good API coverage (default, active, and comment variations). After the import fix, these should serve as solid docs.
Also applies to: 69-91, 93-123
* dev: toolbar component added to propel * dev: toolbar story added * chore: propel config updated * chore: code refactor --------- Co-authored-by: sriram veeraghanta <veeraghanta.sriram@gmail.com>
Description
This PR introduces the toolbar component in propel.
Media
Summary by CodeRabbit
New Features
Documentation
Chores