-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Update index.md #10382
Update index.md #10382
Conversation
centerRight throws of the alignment of the icon with the background of the button on hover.
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. |
The code you are changing here is excerpted from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs the matching source file updated
Thanks @domesticmouse, I've updated the source file(s). |
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL @khanhnwin
Visit the preview URL for this PR (updated for commit 56e35af): |
/gcbrun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. Thanks, @jaredleishman!
_Description of what this PR is changing or adding, and why:_ `Alignment.centerRight` throws off the alignment of the icon with the background of the button on hover. When changing the alignment to `Alignment.center`, the gap between the `IconButton` and `_favoriteCount` is a bit bigger. Ultimately I think merging this change is a matter of opinion and take no offense if we keep the existing alignment. NOTE: This was targeting Linux desktop, not mobile. _Issues fixed by this PR (if any):_ #10381 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Brett Morgan <brettmorgan@google.com>
_Description of what this PR is changing or adding, and why:_ `Alignment.centerRight` throws off the alignment of the icon with the background of the button on hover. When changing the alignment to `Alignment.center`, the gap between the `IconButton` and `_favoriteCount` is a bit bigger. Ultimately I think merging this change is a matter of opinion and take no offense if we keep the existing alignment. NOTE: This was targeting Linux desktop, not mobile. _Issues fixed by this PR (if any):_ flutter#10381 ## Presubmit checklist - [x] This PR doesn’t contain automatically generated corrections (Grammarly or similar). - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style) — for example, it doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: Brett Morgan <brettmorgan@google.com>
Description of what this PR is changing or adding, and why:
Alignment.centerRight
throws off the alignment of the icon with the background of the button on hover. When changing the alignment toAlignment.center
, the gap between theIconButton
and_favoriteCount
is a bit bigger. Ultimately I think merging this change is a matter of opinion and take no offense if we keep the existing alignment.NOTE: This was targeting Linux desktop, not mobile.
Issues fixed by this PR (if any): #10381
Presubmit checklist