Skip to content

Conversation

@mossmana
Copy link
Contributor

@mossmana mossmana commented Dec 6, 2024

Screenshot 2024-12-06 at 1 00 16 PM Screenshot 2024-12-06 at 1 00 28 PM

#8216

Please add a note to packages/devtools_app/release_notes/NEXT_RELEASE_NOTES.md if your change requires release notes. Otherwise, add the 'release-notes-not-required' label to the PR.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read the [Flutter Style Guide] recently, and have followed its advice.
  • 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 ///).
  • I added new tests to check the change I am making, or there is a reason for not adding tests.

![build.yaml badge]

@mossmana mossmana requested a review from a team as a code owner December 6, 2024 21:01
@mossmana mossmana requested review from kenzieschmoll and removed request for a team December 6, 2024 21:01
@kenzieschmoll
Copy link
Member

DBC: instead of including "View Licenses" as a dialog action, can it just be another section of the about dialog similar to how Contributing is displayed?

additionally, do we need to display the licenses as part of the DevTools web app? Or is linking out to the license file on our repo sufficient?

@mossmana
Copy link
Contributor Author

mossmana commented Dec 9, 2024

DBC: instead of including "View Licenses" as a dialog action, can it just be another section of the about dialog similar to how Contributing is displayed?

additionally, do we need to display the licenses as part of the DevTools web app? Or is linking out to the license file on our repo sufficient?

Per offline discussion, it was decided to use the same behavior as the "View Licenses" button in the Flutter AboutDialog.

@kenzieschmoll
Copy link
Member

Please add an entry to NEXT_RELEASE_NOTES.md to get the release notes check to pass.

Comment on lines 148 to 155
/// Since [DevToolsAboutDialog] is not actually an [AboutDialog], there is no
/// built-in way to add a 'View Licenses' button.
/// So, adding a custom [DialogTextButton] to view licenses.
/// Since this action is very specific to just the [DevToolsAboutDialog],
/// providing implementation in about_dialog.dart and not in
/// dialogs.dart which contains the definition of [DevToolsDialog].
/// TODO(mossmana): We may want to consider refactoring [DevToolsAboutDialog] to be an [AboutDialog].
/// https://api.flutter.dev/flutter/material/AboutDialog-class.html
Copy link
Member

Choose a reason for hiding this comment

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

this should use // instead of /// since it is not dart doc for the class, but rather comments to explain why we used this approach.

@mossmana mossmana merged commit 6b53654 into flutter:master Dec 10, 2024
24 checks passed
@mossmana mossmana deleted the 8216-about-dialog branch December 10, 2024 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants