Skip to content

Add guidance for adapting alert dialogs#8429

Merged
sfshaza2 merged 7 commits intomainfrom
alert-dialog-adaptation
Apr 6, 2023
Merged

Add guidance for adapting alert dialogs#8429
sfshaza2 merged 7 commits intomainfrom
alert-dialog-adaptation

Conversation

@InMatrix
Copy link
Copy Markdown
Contributor

@InMatrix InMatrix commented Mar 23, 2023

Description of what this PR is changing or adding, and why:

This PR adds guidance for alert dialogs to Platform-specific behaviors and adaptations
. This is the first of a few cross-platform adaptation guides we're developing. The goal is to address #8427 over time.

Issues fixed by this PR (if any):

#8427

Presubmit checklist

This PR adds guidance for alert dialogs to https://docs.flutter.dev/resources/platform-adaptations. This is the first of a few cross-platform adaptation guides we're developing.
@InMatrix
Copy link
Copy Markdown
Contributor Author

I'm submitting this PR to get this on the radar of the DevRel team and other interested parties. There are some stylistic issues I need to address to satisfy the checklist.

If this is not the right place for the proposed content, please suggest alternatives. Thanks!

Cc: @leighajarett @Nancyhu @HansMuller

@leighajarett
Copy link
Copy Markdown
Contributor

Added @MitchellGoodwin to provide any eng guidance

@InMatrix
Copy link
Copy Markdown
Contributor Author

This PR is ready for review. I'd appreciate if one of the code owners can take a look.

Comment thread src/resources/platform-adaptations.md Outdated
Comment thread src/resources/platform-adaptations.md Outdated
Copy link
Copy Markdown
Contributor

@sfshaza2 sfshaza2 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 updating this, @InMatrix! Ultimately, I prefer that we don't link to personal GitHub accounts from the official docs. I'll leave it for now, but maybe we could finalize the multiplatform version of Veggie Seasons and add it to the examples repo? (Without deleting the original Cupertino version, as I rely on that in the restore state docs?)

Also, we avoid linking to gist files, which aren't tested. Our samples repo undergoes testing on every CI (I believe). Also, our code excerpts are routinely tested on every submit.

Finally, I made the link text more descriptive.

@InMatrix
Copy link
Copy Markdown
Contributor Author

InMatrix commented Mar 28, 2023

@sfshaza2 Thank you for your review!

Ultimately, I prefer that we don't link to personal GitHub accounts from the official docs.

Yes, I'd like to move those platform adaptation guides to an official repo eventually. They're preliminary recommendations at the moment, so I considered two options 1) writing them in. the wiki section of the flutter/flutter repo, and 2) posting them to the Discussion section of the veggieseasons_adaptive repo under my personal account. I went with the second option, because it includes an easy way to receive feedback. Would #1 work better from your perspective? I can link an issue or a survey form from the wiki page as a way to solicit feedback.

maybe we could finalize the multiplatform version of Veggie Seasons and add it to the examples repo? (Without deleting the original Cupertino version, as I rely on that in the restore state docs?)

Yes, this is something I'd like to do in Q2. The multiplatform version won't use state restoration APIs for simplicity.

Also, we avoid linking to gist files, which aren't tested.

That makes sense. Including the full snippet inline is a bit long. I considered using DartPad, but this piece of code doesn't work on the Web. Do you have any suggestions on where I should host this snippet?

Copy link
Copy Markdown
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

Once you address that last comment, lgtm.

@InMatrix
Copy link
Copy Markdown
Contributor Author

Thanks @sfshaza2. I'm making some additional tweaks after getting feedback from @Nancyhu2023. I'll ping this thread once it's ready.

@InMatrix
Copy link
Copy Markdown
Contributor Author

InMatrix commented Apr 5, 2023

@sfshaza2 I updated the PR. Please take another look. Thank you for your help.

Copy link
Copy Markdown
Contributor

@sfshaza2 sfshaza2 left a comment

Choose a reason for hiding this comment

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

lgtm

@sfshaza2 sfshaza2 merged commit 4b47bc6 into main Apr 6, 2023
@sfshaza2 sfshaza2 deleted the alert-dialog-adaptation branch April 6, 2023 00:11
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.

3 participants