Skip to content

Use null-aware elements in NavigationRail#186174

Draft
faheemabbas766 wants to merge 5 commits into
flutter:masterfrom
faheemabbas766:flutter-null-aware-navigation-rail
Draft

Use null-aware elements in NavigationRail#186174
faheemabbas766 wants to merge 5 commits into
flutter:masterfrom
faheemabbas766:flutter-null-aware-navigation-rail

Conversation

@faheemabbas766

@faheemabbas766 faheemabbas766 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Adopts Dart null-aware elements in NavigationRail for the optional trailing widget.

This replaces explicit widget.trailing != null checks followed by widget.trailing! with null-aware collection elements. The behavior stays the same, but the code is cleaner and follows the migration requested in the tracking issue.

Part of #172188

Tests

Ran:

./bin/flutter test packages/flutter/test/material/navigation_rail_test.dart

All 106 tests passed.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///), or this change does not require documentation updates.
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported, or this change is not a breaking change.
  • All existing and new tests are passing.

@flutter-dashboard

flutter-dashboard Bot commented May 7, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels May 7, 2026
@google-cla

google-cla Bot commented May 7, 2026

Copy link
Copy Markdown

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.

gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 7, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request simplifies the rendering of the trailing widget in the NavigationRail by utilizing null-aware element syntax, which replaces manual null checks and assertions. I have no feedback to provide.

@faheemabbas766 faheemabbas766 force-pushed the flutter-null-aware-navigation-rail branch from 688d89c to a01e354 Compare May 7, 2026 07:06
@faheemabbas766

faheemabbas766 commented May 7, 2026

Copy link
Copy Markdown
Contributor Author

CLA is now passing. The remaining failure is the Material/Cupertino code freeze check because this PR touches packages/flutter/lib/src/material/navigation_rail.dart.

This is a small cleanup-only change for null-aware collection elements and does not change runtime behavior, so I am happy to wait until the freeze is lifted if needed.

@victorsanni victorsanni added the CICD Run CI/CD label May 8, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label May 8, 2026
@dkwingsmt dkwingsmt self-requested a review May 13, 2026 18:11
@victorsanni victorsanni added the CICD Run CI/CD label May 21, 2026
dkwingsmt
dkwingsmt approved these changes May 21, 2026
View reviewed changes

@dkwingsmt dkwingsmt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dkwingsmt

dkwingsmt commented May 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution! This PR has reached completion, however we're not able to merge this PR for the moment due to the Material/Cupertino code freeze (#184093). As soon as the decoupling is complete, we will post a way to port this PR to the new packages to land. In the meantime, I'll convert this PR to a draft and apply a label so that we can quickly find it when the freeze is lifted.

@dkwingsmt dkwingsmt marked this pull request as draft May 27, 2026 18:08
@dkwingsmt dkwingsmt added the waiting for code freeze This PR is waiting for a code freeze to resolve. label May 27, 2026
@victorsanni victorsanni mentioned this pull request May 28, 2026
Closed
@Piinks Piinks added the Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. label Jun 24, 2026
@Piinks

Piinks commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

I've marked this PR as not ready to port to flutter/packages yet.
We'll provide instructions to move this change over to material_ui/cupertino_ui once ready to receive PRs. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkwingsmt dkwingsmt dkwingsmt approved these changes
@victorsanni victorsanni victorsanni approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD Decoupling: Not ready to port yet Instructions will be provided when this is ready to move to flutter/packages. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. waiting for code freeze This PR is waiting for a code freeze to resolve.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@faheemabbas766 @dkwingsmt @Piinks @victorsanni @alpesh76