Skip to content

Review current ResponsiveRow implementation #6368

@ndonkoHenri

Description

@ndonkoHenri

Executive Summary

A thorough review of the ResponsiveRow implementation across the Python SDK and Flutter/Dart client revealed 4 bugs, 4 inconsistencies, and 1 minor documentation issue. The most critical issues involve incorrect breakpoint resolution for run_spacing, duplicate widget keys in the child loop, and silent behavior changes when content wraps.

Summary of Findings

# Severity Location Description
1 Bug responsive_row.dart:69 runSpacing uses view.breakpoints instead of local custom breakpoints
2 Bug responsive_row.dart:58 ControlWidget(key: key) assigns parent key to every child in loop
3 Bug responsive_row.dart:73 STRETCH/BASELINE silently fall back to start when content wraps
4 Bug responsive_row.dart:68,78 bpSpacing - 0.1 magic constant with no documentation
5 Inconsistency responsive_row.dart:65-85 alignment semantics silently change between Row and Wrap modes
6 Doc error responsive_row.py:73 spacing note incorrectly claims only effective with 3 alignment modes
7 Inconsistency responsive.dart:39 Tie-breaking in getBreakpointNumber uses >= instead of >
8 Doc error control.py:67 col breakpoint table mixes upper/lower bound conventions
9 Minor responsive_row.py:74 "virtual pixels" should be "logical pixels"

Detailed Findings

Bug 1: runSpacing uses wrong breakpoints

File: responsive_row.dart, line 69

Severity: Bug
The runSpacing calculation passes view.breakpoints (the page-level defaults) instead of the local breakpoints variable built from the control's custom breakpoints (lines 31–37). Every other responsive value (bpSpacing, bpColumns, bpCol) correctly uses the local variable. If a user sets custom breakpoints on their ResponsiveRow, run_spacing will silently be evaluated against the wrong breakpoint set.

Recommended fix: Change view.breakpoints to breakpoints on line 69.

Bug 2: Duplicate widget keys on children

File: responsive_row.dart, line 58

Severity: Bug
ControlWidget(key: key, control: ctrl) reuses the parent widget's key (super.key) for every iteration of the child loop. If super.key is non-null, this assigns duplicate keys to all children, which is a Flutter framework invariant violation and can cause unpredictable rendering behavior.

Recommended fix: Replace key: key with key: ValueKey(ctrl.id) to give each child a unique key.

Bug 3: STRETCH/BASELINE silently break in Wrap mode

File: responsive_row.dart, lines 73–74; types.py, lines 314–320

Severity: Bug

The Python vertical_alignment property is typed as CrossAxisAlignment, which includes STRETCH and BASELINE. When children overflow the column count (totalCols > bpColumns), the Dart code switches to a Wrap widget and parses the same string as WrapCrossAlignment. However, WrapCrossAlignment only supports start, end, and center. The values "stretch" and "baseline" parse to null and silently fall back to WrapCrossAlignment.start. The layout behavior changes without any error the moment content starts wrapping.

Recommended fix: Either restrict the Python type to only values valid in both modes, or add explicit handling in Dart to map STRETCH/BASELINE to sensible Wrap equivalents with a dev warning.

Bug 4: bpSpacing - 0.1 magic constant

File: responsive_row.dart, lines 68 and 78

Severity: Bug

Both the Wrap and Row widgets subtract 0.1 from the spacing value. This appears to be a floating-point workaround to prevent ConstrainedBox widths from summing to just over maxWidth, causing unintended wrapping. However, it means the actual rendered spacing is always 0.1 logical pixels less than what the user configured. There is no code comment explaining this, and it affects both layout modes.

Recommended fix: Add a comment explaining the workaround, or use a more precise approach such as rounding column widths down to avoid the overflow.

Inconsistency 5: alignment semantics differ between Row and Wrap

File: responsive_row.dart, lines 65–85

Severity: Inconsistency

The Python alignment property is typed as MainAxisAlignment (Row semantics), but in Wrap mode the Dart code parses it as WrapAlignment. While the string values overlap (start, end, center, spaceBetween, spaceAround, spaceEvenly), the semantics differ. For example, spaceEvenly in Row distributes free space uniformly including before the first and after the last child, while in Wrap it distributes space between runs, not between individual children within a run. There is no documentation warning users about this behavior change.

**Recommended fix:**Document the behavioral difference, or normalize semantics across both modes.

Inconsistency 6: spacing docstring is incorrect

File: responsive_row.py, lines 73–77

Severity: Doc error

The docstring states that spacing only has effect when alignment is set to START, END, or CENTER. This is incorrect. Flutter's Row.spacing (added in Flutter 3.27) applies in all alignment modes — for spaceAround/spaceBetween/spaceEvenly it adds extra spacing on top of the distribution gaps. Wrap.spacing also always applies regardless of alignment.

Recommended fix: Remove or correct the note in the docstring.

Inconsistency 7: Non-deterministic breakpoint tie-breaking

File: responsive.dart, line 39

Severity: Inconsistency

The getBreakpointNumber function uses >= (not >) when comparing breakpoint widths, so if two breakpoints share the same minimum width, the last one in Map iteration order wins. Since breakpoints come from user-supplied data deserialized into a Dart Map, iteration order may not be guaranteed, making the result non-deterministic.

Recommended fix: Use > instead of >= for the second comparison to make first-registered breakpoint win.

Doc error 8: col breakpoint table is inconsistent

File: control.py, lines 67–77

Severity: Doc error

The breakpoint reference table for the col property uses mixed conventions: the xs row says <576px (an upper bound) while sm says ≥576px (a minimum width). Since breakpoints are minimum widths, xs should say ≥0px. The ResponsiveRowBreakpoint docstrings correctly say "Default min width: 0 px" for XS, so the table is inconsistent with the rest of the documentation.

Recommended fix: Update the table to consistently use minimum widths or explicit ranges.

Minor 9: "virtual pixels" terminology

File: responsive_row.py, line 74

Severity: Minor

The spacing docstring uses the term "virtual pixels." Flutter/Dart uses the standard term "logical pixels" (device-independent pixels). Using non-standard terminology can confuse users who are familiar with Flutter conventions.

Recommended fix: Replace "virtual pixels" with "logical pixels."

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions