Skip to content

feat(genui): Add padding to some items in core catalog #589

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.

Sign up for GitHub

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

Draft
andrewkolos wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat(genui): Add padding to some items in core catalog #589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion examples/simple_chat/lib/main.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,24 @@ ${GenUiPromptFragments.basicChat}''';
};

return Scaffold(
appBar: AppBar(title: Text(title)),
appBar: AppBar(
title: Text(title),
actions: [
Row(
children: [
const Text('Padding'),
Switch(
value: DebugFlags.enableLeafPadding,
onChanged: (value) {
setState(() {
DebugFlags.enableLeafPadding = value;
});
},
),
],
),
Comment on lines +151 to +163
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The Row widget here is redundant because the AppBar's actions property already arranges its children in a horizontal row. You can simplify the code by placing the Text and Switch widgets directly into the actions list.

          const Text('Padding'),
          Switch(
            value: DebugFlags.enableLeafPadding,
            onChanged: (value) {
              setState(() {
                DebugFlags.enableLeafPadding = value;
              });
            },
          ),

],
),
body: SafeArea(
child: Column(
children: [
Expand Down
1 change: 1 addition & 0 deletions packages/genui/lib/genui.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
library;

export 'src/catalog/core_catalog.dart';
export 'src/catalog/core_widgets/widget_helpers.dart';
export 'src/content_generator.dart';
export 'src/core/a2ui_message_processor.dart';
export 'src/core/genui_surface.dart';
Expand Down
2 changes: 2 additions & 0 deletions packages/genui/lib/src/catalog/core_widgets/button.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import '../../model/catalog_item.dart';
import '../../model/ui_models.dart';
import '../../primitives/logging.dart';
import '../../primitives/simple_items.dart';
import 'widget_helpers.dart';

final _schema = S.object(
properties: {
Expand Down Expand Up @@ -87,6 +88,7 @@ final button = CatalogItem(
foregroundColor: primary
? colorScheme.onPrimary
: colorScheme.onSurface,
padding: DebugFlags.enableLeafPadding ? EdgeInsets.zero : null,
).copyWith(textStyle: WidgetStatePropertyAll(textStyle)),
onPressed: () {
final JsonMap resolvedContext = resolveContext(
Expand Down
11 changes: 7 additions & 4 deletions packages/genui/lib/src/catalog/core_widgets/card.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:json_schema_builder/json_schema_builder.dart';
import '../../model/a2ui_schemas.dart';
import '../../model/catalog_item.dart';
import '../../primitives/simple_items.dart';
import 'widget_helpers.dart';

final _schema = S.object(
properties: {'child': A2uiSchemas.componentReference()},
Expand Down Expand Up @@ -37,10 +38,12 @@ final card = CatalogItem(
final cardData = _CardData.fromMap(itemContext.data as JsonMap);
return Card(
color: Theme.of(itemContext.buildContext).colorScheme.surface,
child: Padding(
padding: const EdgeInsets.all(8.0),
child: itemContext.buildChild(cardData.child),
),
child: DebugFlags.enableLeafPadding
? itemContext.buildChild(cardData.child)
: Padding(
padding: const EdgeInsets.all(8.0),
child: itemContext.buildChild(cardData.child),
),
Comment on lines +41 to +46
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The conditional logic here can be confusing. When enableLeafPadding is true, the Padding is removed, which is the opposite of what one might expect from the flag's name. A similar pattern is used in button.dart. While this is likely intentional to avoid double padding when child widgets have their own padding, the inverted logic makes the code harder to understand at a glance. To improve clarity, it would be helpful to add a comment explaining why the padding is being removed when this flag is active.

);
},
exampleData: [
Expand Down
8 changes: 8 additions & 0 deletions packages/genui/lib/src/catalog/core_widgets/image.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import '../../model/a2ui_schemas.dart';
import '../../model/catalog_item.dart';
import '../../primitives/logging.dart';
import '../../primitives/simple_items.dart';
import 'widget_helpers.dart';

Schema _schema({required bool enableUsageHint}) {
final Map<String, Schema> properties = {
Expand Down Expand Up @@ -127,6 +128,13 @@ CatalogItem _imageCatalogItem({
_ => 150.0,
};

if (DebugFlags.enableLeafPadding) {
return Padding(
padding: kDefaultLeafComponentPadding,
child: SizedBox(width: size, height: size, child: child),
);
}

return SizedBox(width: size, height: size, child: child);
},
);
Expand Down
21 changes: 21 additions & 0 deletions packages/genui/lib/src/catalog/core_widgets/text.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import '../../core/widget_utilities.dart';
import '../../model/a2ui_schemas.dart';
import '../../model/catalog_item.dart';
import '../../primitives/simple_items.dart';
import 'widget_helpers.dart';

extension type _TextData.fromMap(JsonMap _json) {
factory _TextData({required JsonMap text, String? usageHint}) =>
Expand Down Expand Up @@ -90,6 +91,26 @@ final text = CatalogItem(
_ => 0.0,
};

if (DebugFlags.enableLeafPadding) {
final EdgeInsets padding = switch (usageHint) {
'h1' || 'h2' || 'h3' => kDefaultLeafComponentPadding.copyWith(
top: 24.0,
bottom: 12.0,
),
_ => kDefaultLeafComponentPadding,
};

return Padding(
padding: padding,
child: MarkdownBody(
data: currentValue ?? '',
styleSheet: MarkdownStyleSheet.fromTheme(
Theme.of(context),
).copyWith(p: baseStyle),
),
);
}

return Padding(
padding: EdgeInsets.symmetric(vertical: verticalPadding),
child: MarkdownBody(
Expand Down
11 changes: 10 additions & 1 deletion packages/genui/lib/src/catalog/core_widgets/text_field.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import '../../model/catalog_item.dart';
import '../../model/data_model.dart';
import '../../model/ui_models.dart';
import '../../primitives/simple_items.dart';
import 'widget_helpers.dart';

final _schema = S.object(
description: 'A text input field.',
Expand Down Expand Up @@ -181,7 +182,7 @@ final textField = CatalogItem(
return ValueListenableBuilder(
valueListenable: labelNotifier,
builder: (context, label, child) {
return _TextField(
final textFieldWidget = _TextField(
initialValue: currentValue ?? '',
label: label,
textFieldType: textFieldData.textFieldType,
Expand Down Expand Up @@ -212,6 +213,14 @@ final textField = CatalogItem(
);
},
);

if (DebugFlags.enableLeafPadding) {
return Padding(
padding: kDefaultLeafComponentPadding,
child: textFieldWidget,
);
}
return textFieldWidget;
},
);
},
Expand Down
9 changes: 9 additions & 0 deletions packages/genui/lib/src/catalog/core_widgets/widget_helpers.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ import '../../model/data_model.dart';
import '../../primitives/logging.dart';
import '../../primitives/simple_items.dart';

const EdgeInsets kDefaultLeafComponentPadding = EdgeInsets.symmetric(
horizontal: 16.0,
vertical: 12.0,
);

class DebugFlags {
static bool enableLeafPadding = true;
}
Comment on lines +17 to +19
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a global static mutable variable like enableLeafPadding can introduce hidden dependencies and make the UI state difficult to reason about, as it can be modified from anywhere. For this experimental feature, it's a quick way to toggle behavior. However, for a more robust and maintainable solution, consider passing this flag down the widget tree using an InheritedWidget or a state management solution (like provider). This would make the dependency explicit and the state changes more controlled. This pattern should be avoided in production code.


/// Builder function for creating a widget from a template and a list of data.
///
/// This is used by [ComponentChildrenBuilder] when children are defined by a
Expand Down
Loading