Skip to content

Added computeDryBaseline implementation in RenderAligningShiftedBox #171250

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.

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

Open
wants to merge 42 commits into
base: master
Choose a base branch
from

Conversation

lewinpauli
Copy link

@lewinpauli lewinpauli commented Jun 26, 2025

Implements computeDryBaseline method in RenderAligningShiftedBox to fix layout failures when DropdownButtonFormField is used inside Wrap inside AlertDialog. The method calculates baseline using dry layout methods and accounts for child positioning within the parent's coordinate space.

Issue appeared when Dropdownfield was used inside a Wrap inside Alertdialog

Fixes flutter/flutter/issues/169214

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Implements computeDryBaseline method in RenderShiftedBox to fix layout
failures when DropdownButtonFormField is used inside Wrap inside AlertDialog.
The method calculates baseline using dry layout methods and accounts for
child positioning within the parent's coordinate space.

Fixes flutter/issues/169214
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

Copy link

google-cla bot commented Jun 26, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the framework flutter/packages/flutter repository. See also f: labels. label Jun 26, 2025
@justinmc
Copy link
Contributor

justinmc commented Jul 8, 2025

Thanks for opening a PR for this! Would you be able to add a test? Ideally something that reproduces the issue in #169214 and shows that it no longer happens with this change.

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@lewinpauli
Copy link
Author

@justinmc while creating tests i came across that "RenderSizedBox" has not implemented method "computeDryBaseline" either, but i don't have a deep flutter framework understanding so i don't know if this is a bug

test('computeDryBaseline returns null when child has no baseline', () {
      final RenderSizedBox child = RenderSizedBox(const Size(50.0, 50.0));
      final RenderPadding padding = RenderPadding(
        padding: const EdgeInsets.all(10.0),
        child: child,
      );

      final double? baseline = padding.computeDryBaseline(
        const BoxConstraints(maxWidth: 100.0, maxHeight: 100.0),
        TextBaseline.alphabetic,
      );

      expect(baseline, isNull);
    });
The RenderSizedBox class does not implement "computeDryBaseline".
If you are not writing your own RenderBox subclass, then this is not
your fault. Contact support: https://github.com/flutter/flutter/issues/new?template=02_bug.yml
package:flutter/src/rendering/box.dart 2229:9          RenderBox.debugCannotComputeDryLayout.<fn>
package:flutter/src/rendering/box.dart 2233:6          RenderBox.debugCannotComputeDryLayout
package:flutter/src/rendering/box.dart 2184:7          RenderBox.computeDryBaseline
package:flutter/src/rendering/box.dart 2140:50         RenderBox._computeDryBaseline
package:flutter/src/rendering/box.dart 1088:42         _Baseline.memoize.ifAbsent
dart:_compact_hash                                     _LinkedHashMapMixin.putIfAbsent
package:flutter/src/rendering/box.dart 1089:18         _Baseline.memoize
package:flutter/src/rendering/box.dart 1620:32         RenderBox._computeWithTimeline
package:flutter/src/rendering/box.dart 1598:26         RenderBox._computeIntrinsics
package:flutter/src/rendering/box.dart 2118:36         RenderBox.getDryBaseline
package:flutter/src/rendering/shifted_box.dart 255:30  RenderPadding.computeDryBaseline
test/rendering/shifted_box_test.dart 52:40             main.<fn>.<fn>
shifted_box_test.dart:52
✖ RenderShiftedBox computeDryBaseline returns null when child has no baseline

@github-actions github-actions bot added the a: desktop Running on desktop label Jul 29, 2025
@lewinpauli
Copy link
Author

lewinpauli commented Jul 30, 2025

@justinmc how can i add a checksum in my pubspec.yaml, that is needed for my integration test. Which algorithm/command is used?

(most of the tests in CI/CD are failing because the checksum does not exist)

locally integration test reproduces the flutter error flutter/flutter/issues/169214 and ensures its working

name: alert_dialog_dropdown_crash
description: An integration test to reproduce AlertDialog with DropdownButton crash.
environment:
  sdk: ^3.8.0-0

# resolution: workspace

dependencies:
  flutter:
    sdk: flutter
  integration_test:
    sdk: flutter
  flutter_test:
    sdk: flutter
  test: any

  meta: any

flutter:
  uses-material-design: true

# PUBSPEC CHECKSUM: ?????

@lewinpauli lewinpauli changed the title Added computeDryBaseline implementation in RenderShiftedBox Added computeDryBaseline implementation in RenderAligningShiftedBox Jul 30, 2025
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

Maybe you don't need an integration test at all?

I'm still curious about the checksum though. I actually don't know how those are generated off the top of my head. I'll look for an answer either way.

Edit: I believe you need to run flutter update-packages --update-hashes to update that checksum. Thanks to @jason-simmons for helping me out with that.

lewinpauli and others added 4 commits August 2, 2025 10:20
…st/material/dropdown_dialog_baseline_test.dart and update computeDryBaseline in RenderAligningShiftedBox to prevent crashes with DropdownButton in AlertDialog
@github-actions github-actions bot added f: material design flutter/packages/flutter/material repository. and removed a: desktop Running on desktop labels Aug 10, 2025
@lewinpauli
Copy link
Author

lewinpauli commented Aug 10, 2025

@justinmc I added missing computeDryBaseline in RenderAlignedShiftedBox

also now a unittest exists: packages/flutter/test/rendering/shifted_box_test.dart

and I replaced the integration test with a much less ressource intensive widget test: packages/flutter/test/rendering/aligning_shifted_box_baseline_test.dart

to verify computeDryBaseline in RenderAlignedShiftedBox exists, and the widget test reproduces the issue in #169214

…uteDryLayout and remove unused aligning_shifted_box_baseline_test.dart in material directory
@github-actions github-actions bot removed the f: material design flutter/packages/flutter/material repository. label Aug 11, 2025
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

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

Thanks for moving the integration tests! It looks like it's thoroughly tested now.

Check out the failures, it looks like there is a legitimate problem with how the dry baseline is calculated.

lewinpauli and others added 13 commits August 12, 2025 16:24
…ftedBox, and refactor size calculations in overflow boxes.
…nedBox, covering various scenarios and edge cases
…s for computeDryBaseline with improved BoxConstraints
…ut(constraints) instead of manually calculating
…consistency also fixed computeDistanceToActualBaseline to make flutter test packages/flutter/test/material/toggle_buttons_test.dart pass
@github-actions github-actions bot added the f: material design flutter/packages/flutter/material repository. label Aug 15, 2025
lewinpauli and others added 5 commits August 15, 2025 15:32
…ine to RenderAnimatedSize for improved layout handling, also _RenderInputPadding _computeSize switched up height & width
rendering/flex:

Add dry baseline simulation helpers mirroring performLayout.
In horizontal baseline mode, align children without baselines to the top (ignore verticalDirection) and mirror in dry path.
rendering/shifted_box:

RenderPadding.computeDryBaseline: return child baseline + padding.top; null if child has none.
RenderSizedOverflowBox.computeDistanceToActualBaseline: include child offset.dy when present.
Tidy base comment in RenderShiftedBox.computeDryBaseline.
rendering/proxy_box:

computeDryBaseline: use child’s dry baseline when available, otherwise fall back to super.
material/badge:

_RenderBadge: implement computeDryLayout and computeDryBaseline to mirror performLayout (alignment, explicit offset, label vertical shift).
material/time_picker:

_RenderInputPadding.computeDryBaseline: compute center offset using dry size and add to child baseline.
semantics/binding:

Safely invoke platformDispatcher.setSemanticsTreeEnabled in tests via dynamic call, ignore when unavailable.
Rationale:

Make computeDryBaseline match actual layout offsets across wrappers.
Fix baseline alignment invariants in Flex and wrappers.
Eliminate “computeDistanceToActualBaseline differs from computeDryBaseline” assertions.
Tests:

rendering/flex “children with no baselines are top-aligned” passes.
material badge/time_picker baseline assertions resolved.
No functional changes to Cupertino filter behavior.

currently only flutter test packages/flutter/test/cupertino/popup_surface_test.dart -p vm --plain-name 'Saturation is applied before blur' fails
@github-actions github-actions bot added the a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) label Aug 20, 2025
@chunhtai chunhtai self-requested a review August 20, 2025 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RenderPositionedBox class does not implement "computeDryBaseline" Alertdialog -> Wrap -> SizedBox -> DropdownButtonFormField
2 participants