Bison card widget (stacked with image variant) - #65
Conversation
Code Coverage Report - 1,260 of 2,814 lines covered ( ⛔ 44.78%)
|
There was a problem hiding this comment.
Pull request overview
Adds the initial BisonCard implementation for the design system, focusing on the “stacked with image” variant requested in issue #10, and wires it into the public API and catalog/example app.
Changes:
- Introduces
BisonCard.stackedWithImagewith header, media, title/subtitle, supporting text, and optional actions. - Exports
BisonCardviacore_widgets.dart, adds a Widgetbook catalog use case, and adds widget tests for basic/optional rendering. - Applies a small formatting update to
BisonDividerand updates example lock/generated directory listing.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/widget/cards/bison_card_test.dart | Adds widget tests for required content and optional actions/trailing icon. |
| lib/src/core_widgets/dividers/bison_divider.dart | Reformats the constructor for readability. |
| lib/src/core_widgets/cards/bison_card.dart | Adds the new BisonCard stacked-with-image implementation and API docs. |
| lib/core_widgets.dart | Exports BisonCard from the core widgets barrel. |
| example/pubspec.lock | Updates example dependency lockfile / SDK constraints. |
| example/lib/main.directories.g.dart | Registers the new cards folder/component in Widgetbook directories (generated). |
| example/lib/bison_card.dart | Adds a Widgetbook use case for interactively previewing BisonCard. |
Suppressed comments (1)
test/widget/cards/bison_card_test.dart:70
- Similar to the previous test,
find.byType(TextButton)is broad and can become flaky if otherTextButtons are introduced elsewhere in the scaffold. Checking for the specific action labels keeps the intent clear and avoids false positives/negatives.
expect(find.byIcon(Icons.more_vert), findsNothing);
expect(find.byType(TextButton), findsNothing);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ), | ||
|
|
||
| // Media area | ||
| ClipRRect(child: SizedBox(width: 360, height: 222, child: media)), |
| final String? supportingText; | ||
|
|
||
| /// Primary action widget displayed in the button row. | ||
| final Widget? primaryAction; |
There was a problem hiding this comment.
I'm guessing a button will always be used for the primary and secondary action. Should we constrain this property to be always a button? We could even constrain to a BisonButton. Just a thought
| final String? subheadText; | ||
|
|
||
| /// Trailing icon button in the header row (e.g. three-dot menu). | ||
| final Widget? trailingIconButton; |
There was a problem hiding this comment.
This is fine I think but if this icon button will always present a menu of items maybe it's better to pass the menu items? If it's empty we don't display the iconButton?
| decoration: BoxDecoration( | ||
| color: bison.theme.surfaceDefault, | ||
| border: Border.all(color: bison.theme.borderPlain), | ||
| borderRadius: BorderRadius.circular(bison.corners.cornerLarge), |
There was a problem hiding this comment.
I think the figma file says the corners should be small
| primaryAction: TextButton(onPressed: () {}, child: Text(primaryLabel)), | ||
| secondaryAction: TextButton(onPressed: () {}, child: Text(secondaryLabel)), |
There was a problem hiding this comment.
The figma spec uses a primary button for the primary action and a outlined button for the secondary action. We should use that. Maybe force that in the API?
There was a problem hiding this comment.
Just a visual issue I noticed in the widgetbook. It looks like the image isn't filling the right side entirely like in Figma, let's make sure it goes end to end.
Also, when I expand the area the widget is in, the widget fills the space. Not sure if that's intentional
Code Coverage Report - 1,321 of 2,970 lines covered ( ⛔ 44.48%)
|
Code Coverage Report - 1,321 of 2,970 lines covered ( ⛔ 44.48%)
|
Code Coverage Report - 1,325 of 2,975 lines covered ( ⛔ 44.54%)
|
Covers the stacked variant only;
Relates to #10