Skip to content
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

compose: Compose-box border is subtle in dark mode #1233

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lakshya1goel
Copy link

@lakshya1goel lakshya1goel commented Dec 29, 2024

Updated the top border and added shadow effect to the top of the compose box to ensure it is subtle in dark mode.

For reference, see the Figma design:
https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=5908-64038&t=alSmAmdRXFDwT4IT-1

Fixes: #1207

Before After
WhatsApp Image 2024-12-29 at 2 28 00 PM WhatsApp Image 2024-12-29 at 2 28 01 PM

@lakshya1goel lakshya1goel marked this pull request as draft December 29, 2024 09:11
Copy link
Collaborator

@chrisbobbe chrisbobbe left a comment

Choose a reason for hiding this comment

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

Thanks! I see this is a draft, but I gave it a quick skim and I noticed a different direction I'd like to take. 🙂

Since the shadow is only meant to be applied in dark theme, I'd like to avoid asking Flutter to do any computations for it when in light mode. For that, let's add a ComposeBoxTheme class in lib/widgets/compose_box.dart, with just one field boxShadow that's null in light theme and non-null in dark theme. ContentTheme and MessageListTheme are examples of the pattern I have in mind. So, something like this:

Proposal
/// Compose-box styles that differ between light and dark theme.
///
/// These styles will animate on theme changes (with help from [lerp]).
class ComposeBoxTheme extends ThemeExtension<ComposeBoxTheme> {
  factory ComposeBoxTheme.light(BuildContext context) {
    return ComposeBoxTheme._(
      boxShadow: null,
    );
  }

  factory ComposeBoxTheme.dark(BuildContext context) {
    return ComposeBoxTheme._(
      boxShadow: [BoxShadow(
        color: DesignVariables.dark.bgTopBar,
        offset: const Offset(0, -4),
        blurRadius: 16,
        spreadRadius: 0,
      )],
    );
  }

  ComposeBoxTheme._({
    required this.boxShadow,
  });

  /// The [ComposeBoxTheme] from the context's active theme.
  ///
  /// The [ThemeData] must include [ComposeBoxTheme] in [ThemeData.extensions].
  static ComposeBoxTheme of(BuildContext context) {
    final theme = Theme.of(context);
    final extension = theme.extension<ComposeBoxTheme>();
    assert(extension != null);
    return extension!;
  }

  final List<BoxShadow>? boxShadow;

  @override
  ComposeBoxTheme copyWith({
    List<BoxShadow>? boxShadow,
  }) {
    return ComposeBoxTheme._(
      boxShadow: boxShadow ?? this.boxShadow,
    );
  }

  @override
  ComposeBoxTheme lerp(ComposeBoxTheme other, double t) {
    if (identical(this, other)) {
      return this;
    }
    return ComposeBoxTheme._(
      boxShadow: BoxShadow.lerpList(boxShadow, other.boxShadow, t)!,
    );
  }
}

Then the caller can say boxShadow: ComposeBoxTheme.of(context).boxShadow.

Note the use of DesignVariables.dark.bgTopBar instead of adding a new design variable. The Figma doesn't add any new design variables. Also, it's DesignVariables.dark.bgTopBar instead of DesignVariables.dark().bgTopBar because of a small optimization, #1236; I suggest rebasing your work atop that PR to get that optimization.

lib/widgets/theme.dart Outdated Show resolved Hide resolved
lib/widgets/theme.dart Outdated Show resolved Hide resolved
@lakshya1goel
Copy link
Author

Thanks for the review, I am working on it and will update it as required ASAP.
Thanks!

Updated the top border and added shadow effect to the top of
the compose box to ensure it is subtle in dark mode.

For reference, see the Figma design:
https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=5908-64038&t=alSmAmdRXFDwT4IT-1

Fixes: zulip#1207
@lakshya1goel lakshya1goel marked this pull request as ready for review January 4, 2025 06:50
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-box border is subtle in dark mode
2 participants