Edit-message (3/n): Implement ZulipWebUiKitButton, for "Cancel" / "Save" buttons #1432
+316
−46
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| import 'package:flutter/material.dart'; | ||
|
|
||
| import 'color.dart'; | ||
| import 'text.dart'; | ||
| import 'theme.dart'; | ||
|
|
||
| /// The "Button" component from Zulip Web UI kit, | ||
| /// plus outer vertical padding to make the touch target 44px tall. | ||
| /// | ||
| /// The Figma uses this for the "Cancel" and "Save" buttons in the compose box | ||
| /// for editing an already-sent message. | ||
| /// | ||
| /// See Figma: | ||
| /// * Component: https://www.figma.com/design/msWyAJ8cnMHgOMPxi7BUvA/Zulip-Web-UI-kit?node-id=1-2780&t=Wia0D0i1I0GXdD9z-0 | ||
| /// * Edit-message compose box: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3988-38201&m=dev | ||
| class ZulipWebUiKitButton extends StatelessWidget { | ||
| const ZulipWebUiKitButton({ | ||
| super.key, | ||
| this.attention = ZulipWebUiKitButtonAttention.medium, | ||
| this.intent = ZulipWebUiKitButtonIntent.info, | ||
| required this.label, | ||
| required this.onPressed, | ||
| }); | ||
|
|
||
| final ZulipWebUiKitButtonAttention attention; | ||
| final ZulipWebUiKitButtonIntent intent; | ||
| final String label; | ||
| final VoidCallback onPressed; | ||
|
|
||
| WidgetStateColor _backgroundColor(DesignVariables designVariables) { | ||
| switch ((attention, intent)) { | ||
| case (ZulipWebUiKitButtonAttention.medium, ZulipWebUiKitButtonIntent.info): | ||
| return WidgetStateColor.fromMap({ | ||
| WidgetState.pressed: designVariables.btnBgAttMediumIntInfoActive, | ||
| ~WidgetState.pressed: designVariables.btnBgAttMediumIntInfoNormal, | ||
| }); | ||
| case (ZulipWebUiKitButtonAttention.high, ZulipWebUiKitButtonIntent.info): | ||
| return WidgetStateColor.fromMap({ | ||
| WidgetState.pressed: designVariables.btnBgAttHighIntInfoActive, | ||
| ~WidgetState.pressed: designVariables.btnBgAttHighIntInfoNormal, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| Color _labelColor(DesignVariables designVariables) { | ||
| switch ((attention, intent)) { | ||
| case (ZulipWebUiKitButtonAttention.medium, ZulipWebUiKitButtonIntent.info): | ||
| return designVariables.btnLabelAttMediumIntInfo; | ||
| case (ZulipWebUiKitButtonAttention.high, ZulipWebUiKitButtonIntent.info): | ||
| return designVariables.btnLabelAttHigh; | ||
| } | ||
| } | ||
|
|
||
| TextStyle _labelStyle(BuildContext context, {required TextScaler textScaler}) { | ||
| final designVariables = DesignVariables.of(context); | ||
| // Values chosen from the Figma frame for zulip-flutter's compose box: | ||
| // https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=3988-38201&m=dev | ||
| // Commented values come from the Figma page "Zulip Web UI kit": | ||
| // https://www.figma.com/design/msWyAJ8cnMHgOMPxi7BUvA/Zulip-Web-UI-kit?node-id=1-8&p=f&m=dev | ||
| // Discussion: | ||
| // https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023880851 | ||
| return TextStyle( | ||
| color: _labelColor(designVariables), | ||
| fontSize: 17, // 16 | ||
| height: 1.20, // 1.25 | ||
| letterSpacing: proportionalLetterSpacing(context, textScaler: textScaler, | ||
| 0.006, | ||
| baseFontSize: 17), // 16 | ||
| ).merge(weightVariableTextStyle(context, | ||
| wght: 600)); // 500 | ||
| } | ||
|
|
||
| BorderSide _borderSide(DesignVariables designVariables) { | ||
| switch (attention) { | ||
| case ZulipWebUiKitButtonAttention.medium: | ||
| // TODO inner shadow effect like `box-shadow: inset`, following Figma; | ||
| // needs Flutter support for something like that: | ||
| // https://github.com/flutter/flutter/issues/18636 | ||
| // https://github.com/flutter/flutter/issues/52999 | ||
| // For now, we just use a solid-stroke border with half the opacity | ||
| // and half the width. | ||
| return BorderSide( | ||
| color: designVariables.btnShadowAttMed.withFadedAlpha(0.5), | ||
| width: 0.5); | ||
| case ZulipWebUiKitButtonAttention.high: | ||
| return BorderSide.none; | ||
| } | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| final designVariables = DesignVariables.of(context); | ||
|
|
||
| // With [MaterialTapTargetSize.padded], | ||
| // make [TextButton] set 44 instead of 48 for the touch-target height. | ||
| final visualDensity = VisualDensity(vertical: -1); | ||
| // A value that [TextButton] adds to some of its layout parameters; | ||
| // we can cancel out those adjustments by subtracting it. | ||
| final densityVerticalAdjustment = visualDensity.baseSizeAdjustment.dy; | ||
|
|
||
| // An upper limit when the text-size setting is large | ||
| // - helps prioritize more important content (like message content); #1023 | ||
| // - prevents the vertical padding added by [MaterialTapTargetSize.padded] | ||
| // from shrinking to zero as the button grows to accommodate a larger label | ||
| final textScaler = MediaQuery.textScalerOf(context).clamp(maxScaleFactor: 1.5); | ||
|
|
||
| return AnimatedScaleOnTap( | ||
| scaleEnd: 0.96, | ||
| duration: Duration(milliseconds: 100), | ||
| child: TextButton( | ||
| style: TextButton.styleFrom( | ||
| padding: EdgeInsets.symmetric(horizontal: 10, vertical: 4 - densityVerticalAdjustment), | ||
| foregroundColor: _labelColor(designVariables), | ||
| shape: RoundedRectangleBorder( | ||
| side: _borderSide(designVariables), | ||
| borderRadius: BorderRadius.circular(4)), | ||
| splashFactory: NoSplash.splashFactory, | ||
|
|
||
| // These three arguments make the button 28px tall vertically, | ||
| // but with vertical padding to make the touch target 44px tall: | ||
| // https://github.com/zulip/zulip-flutter/pull/1432#discussion_r2023907300 | ||
| visualDensity: visualDensity, | ||
| tapTargetSize: MaterialTapTargetSize.padded, | ||
| minimumSize: Size(kMinInteractiveDimension, 28 - densityVerticalAdjustment), | ||
| ).copyWith(backgroundColor: _backgroundColor(designVariables)), | ||
| onPressed: onPressed, | ||
| child: ConstrainedBox( | ||
| constraints: BoxConstraints(maxWidth: 240), | ||
|
There was a problem hiding this comment. I wonder if we need a different value for the mobile app. Presumably this is for web. There was a problem hiding this comment. If 240 doesn't seem OK, could you say more about why? If it's OK but just not ideal, I think it's fine to leave it as-is and tune later based on feedback or more design work. There was a problem hiding this comment. Usually when we have wider-than-usual buttons, they occupy the screen-width or the width of the parent; for my phone a 240px button would be a bit narrower than my screen. I think leaving it this way and tuning later sounds like a good plan. |
||
| child: Text(label, | ||
| textScaler: textScaler, | ||
| maxLines: 1, | ||
| style: _labelStyle(context, textScaler: textScaler), | ||
| textAlign: TextAlign.center, | ||
| overflow: TextOverflow.ellipsis)))); | ||
| } | ||
| } | ||
|
|
||
| enum ZulipWebUiKitButtonAttention { | ||
| high, | ||
| medium, | ||
| // low, | ||
| } | ||
|
|
||
| enum ZulipWebUiKitButtonIntent { | ||
| // neutral, | ||
| // warning, | ||
| // danger, | ||
| info, | ||
| // success, | ||
| // brand, | ||
|
Comment on lines
+144
to
+150
There was a problem hiding this comment. One good reason to add these only lazily, as you're doing here: to keep down how many different colors we have to maintain in theme.dart, until we finish #831. |
||
| } | ||
|
|
||
| /// Apply [Transform.scale] to the child widget when tapped, and reset its scale | ||
| /// when released, while animating the transitions. | ||
| class AnimatedScaleOnTap extends StatefulWidget { | ||
| const AnimatedScaleOnTap({ | ||
| super.key, | ||
| required this.scaleEnd, | ||
| required this.duration, | ||
| required this.child, | ||
| }); | ||
|
|
||
| /// The terminal scale to animate to. | ||
| final double scaleEnd; | ||
|
|
||
| /// The duration over which to animate the scale change. | ||
| final Duration duration; | ||
|
|
||
| final Widget child; | ||
|
|
||
| @override | ||
| State<AnimatedScaleOnTap> createState() => _AnimatedScaleOnTapState(); | ||
| } | ||
|
|
||
| class _AnimatedScaleOnTapState extends State<AnimatedScaleOnTap> { | ||
| double _scale = 1; | ||
|
|
||
| void _changeScale(double scale) { | ||
| setState(() { | ||
| _scale = scale; | ||
| }); | ||
| } | ||
|
|
||
| @override | ||
| Widget build(BuildContext context) { | ||
| return GestureDetector( | ||
| behavior: HitTestBehavior.translucent, | ||
| onTapDown: (_) => _changeScale(widget.scaleEnd), | ||
| onTapUp: (_) => _changeScale(1), | ||
| onTapCancel: () => _changeScale(1), | ||
| child: AnimatedScale( | ||
| scale: _scale, | ||
| duration: widget.duration, | ||
| curve: Curves.easeOut, | ||
| child: widget.child)); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Cool, glad you found a way to control this.