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

Adjust MarkAsReadWidget text to be bolder, use Source Sans 3 #376

Merged
merged 2 commits into from
Nov 16, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 18 additions & 9 deletions lib/widgets/message_list.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import 'page.dart';
import 'profile.dart';
import 'sticky_header.dart';
import 'store.dart';
import 'text.dart';

class MessageListPage extends StatefulWidget {
const MessageListPage({super.key, required this.narrow});
Expand Down Expand Up @@ -406,12 +407,18 @@ class MarkAsReadWidget extends StatelessWidget {
// TODO(#368): this should pull from stream color
color: Colors.transparent,
child: Padding(
padding: const EdgeInsets.all(10),
// vertical padding adjusted for tap target height (48px) of button
padding: const EdgeInsets.symmetric(horizontal: 10, vertical: 10 - ((48 - 38) / 2)),
child: FilledButton.icon(
style: FilledButton.styleFrom(
padding: const EdgeInsets.all(10),
textStyle: const TextStyle(fontSize: 18, fontWeight: FontWeight.w200),
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(5)),
backgroundColor: _UnreadMarker.color,
Copy link
Member

Choose a reason for hiding this comment

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

msglist [nfc]: Factor out color for _UnreadMarker and MarkAsReadWidget

This change isn't NFC, because it changes the color of MarkAsReadWidget.

Could have an NFC commit without this line, just pulling out the static _UnreadMarker.color, and then a second commit to use it here. Or this commit is fine as is, it just isn't NFC (and what it's doing isn't "factoring out" from MarkAsReadWidget, because this color wasn't previously there) — a good commit message would be msglist: Color MarkAsReadWidget the same as _UnreadMarker.

minimumSize: const Size.fromHeight(38),
textStyle: const TextStyle(
fontFamily: 'Source Sans 3',
fontSize: 18,
height: (23 / 18),
).merge(weightVariableTextStyle(context)),
shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(7)),
),
onPressed: () => _handlePress(context),
icon: const Icon(Icons.playlist_add_check),
Expand Down Expand Up @@ -467,6 +474,12 @@ class _UnreadMarker extends StatelessWidget {
final bool isRead;
final Widget child;

// The color hsl(227deg 78% 59%) comes from the Figma mockup at:
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=132-9684
// See discussion about design at:
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20unread.20marker/near/1658008
static final color = const HSLColor.fromAHSL(1, 227, 0.78, 0.59).toColor();

@override
Widget build(BuildContext context) {
return Stack(
Expand All @@ -485,11 +498,7 @@ class _UnreadMarker extends StatelessWidget {
curve: Curves.easeOut,
child: DecoratedBox(
decoration: BoxDecoration(
// The color hsl(227deg 78% 59%) comes from the Figma mockup at:
// https://www.figma.com/file/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=132-9684
// See discussion about design at:
// https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/flutter.3A.20unread.20marker/near/1658008
color: const HSLColor.fromAHSL(1, 227, 0.78, 0.59).toColor(),
color: color,
// TODO(#95): Don't show this extra border in dark mode, see:
// https://github.com/zulip/zulip-flutter/pull/317#issuecomment-1784311663
border: Border(left: BorderSide(
Expand Down