Skip to content

compose: Shrink "send" button to match content-input height, like before M3 #399

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

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

chrisbobbe
Copy link
Collaborator

The way to get the smaller IconButton height we want has changed. After migrating to Material Design 3, in #380, we have to do this different thing.

Fixes: #398

@chrisbobbe chrisbobbe requested a review from gnprice November 20, 2023 18:28
@chrisbobbe
Copy link
Collaborator Author

Before After

…ore M3

The way to get the smaller IconButton height we want has changed.
After migrating to Material Design 3, in zulip#380, we have to do this
different thing.

Fixes: zulip#398
@gnprice
Copy link
Member

gnprice commented Nov 21, 2023

Thanks for the fix!

Merging, after adjusting the comment to be more definitive:

         style: const ButtonStyle(
-          // Match the height of the content input. Empirically, setting
-          // [MaterialTapTargetSize.shrinkWrap] seems necessary to let the size
-          // be smaller than 48px square (at least without messing with
-          // [visualDensity]). So do that. It would be nice if the touch target
-          // extended invisibly out from the button, to make a 48px square,
-          // but I can't easily check that in the inspector. It might be the
-          // behavior, from reading docs, but the docs aren't clear enough
-          // to rely on.
+          // Match the height of the content input.
           minimumSize: MaterialStatePropertyAll(Size.square(_sendButtonSize)),
+          // With the default of [MaterialTapTargetSize.padded], not just the
+          // tap target but the visual button would get padded to 48px square.
+          // It would be nice if the tap target extended invisibly out from the
+          // button, to make a 48px square, but that's not the behavior we get.
           tapTargetSize: MaterialTapTargetSize.shrinkWrap,

@gnprice gnprice force-pushed the pr-shrink-send-button-after-m3 branch from b47873f to c0a1064 Compare November 21, 2023 00:45
@gnprice gnprice merged commit c0a1064 into zulip:main Nov 21, 2023
PIG208 added a commit to PIG208/zulip-flutter that referenced this pull request Nov 15, 2024
- We drop `_sendButtonSize` and `_inputVerticalPadding` because we no
  longer need them for setting the button's minHeight, along with
  `ButtonStyle` for the send button that was added in zulip#399, which is
  irrelevant to the new design.

- `ClipRect`'s size is determined by the `ConstrainedBox`.  This is
  mainly for showing the content through the `contentPadding` of the
  `TextField`, so that our `InsetShadowBox` can fade it smoothly there.
  The shadow is always there, but it is only visible when the
  `TextField` is long enough to be scrollable. Discussion here:
    zulip#928 (comment)

- For `InputDecorationTheme` on `_ComposeBoxLayout`, we zero out
  `contentPadding` while keeping `isDense` as `true`, to explicitly
  remove paddings on the input widgets.

- The height of the compose buttons is 42px in the Figma design, but
  44px in the implementation.  We change that to match the minimum
  button size per the accessibility recommendation from Apple.
  Discussion here:
    zulip#928 (comment)

- Note that we use `withFadedAlpha` on `designVariables.textInput` because
  the color is already transparent in dark mode, and the helper allows us
  to multiply, instead of to override, the alpha channel of the color with
  a factor.  Discussion here:
    zulip#928 (comment)

- DesignVariables.icon's value has been updated to match the current
  design.  This would affect the appearance of the
  ChooseAccountPageOverflowButton on the choose account page, which is
  intentional.

This is "most of" the redesign because the new button feedback is
supported later.

Design spec here:
  - https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3954-13395
  - https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3862-14350

Signed-off-by: Zixuan James Li <[email protected]>
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.

compose: Send button got larger than content-input height in Material 3 migration
2 participants