Skip to content

Star icon too high on messages #616

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.

Sign up for GitHub

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

Jump to bottom
Closed
gnprice opened this issue Apr 3, 2024 · 1 comment · Fixed by #762
Closed

Star icon too high on messages #616

gnprice opened this issue Apr 3, 2024 · 1 comment · Fixed by #762
Labels
a-design Visual and UX design a-msglist The message-list screen, except what's label:a-content beta feedback Things beta users have specifically asked for
Milestone
M3: Summer 2024

Comments

@gnprice
Copy link
Member

gnprice commented Apr 3, 2024

In the message list, when a message is starred, the star icon appears too high relative to the message text:
image

See chat thread for discussion.

Instead, the "shoulders" of the star (the near-horizontal top edges of the left and right wedges of the star) should be at about the x-height of the first line of text, i.e. about the same height as the top of letters like "x" and "e". Like this (from this PR subthread):
image
@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-design Visual and UX design beta feedback Things beta users have specifically asked for labels Apr 3, 2024
@gnprice gnprice added this to the Beta 2 milestone Apr 3, 2024
@Syrineladeb02 Syrineladeb02 mentioned this issue Apr 22, 2024
Closed
@Syrineladeb02 Syrineladeb02 mentioned this issue May 8, 2024
Closed
@PIG208
Copy link
Member

PIG208 commented Jun 24, 2024 •
edited
Loading

A side by side comparison when the system font size changes using the configuration from #625 where the top inset of the star is set to 5.5 (originally 4):

image

This indicates that neither will work, because the insets do not scale with the text. Ideally we want to make sure the alignment is responsive regardless of the font size. So using a constant is likely not an ideal solution.

@PIG208 PIG208 linked a pull request Jun 26, 2024 that will close this issue
Show marked/edited markers, without swipe gesture #762
Merged
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 26, 2024
@PIG208
star: Align the icon with message content.
8c2ae12 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 26, 2024
@PIG208
star: Align the icon with message content.
cf311bd 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 26, 2024
@PIG208
star: Align the icon with message content.
ac3770d 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 26, 2024
@PIG208
star: Align the icon with message content.
44c75d2 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 27, 2024
@PIG208
star: Align the icon with message content.
7b49c2a 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 28, 2024
@PIG208
star: Align the icon with message content.
dbadc55 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jun 28, 2024
@PIG208
star: Align the icon with message content.
1681a97 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 1, 2024
@PIG208
star: Align the icon with message content.
3d2eb14 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 3, 2024
@PIG208
star: Align the icon with message content.
7c33eac 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 3, 2024
@PIG208
star: Align the icon with message content.
a196adc 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 3, 2024
@PIG208
star: Align the icon with message content.
5325cbb 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 3, 2024
@PIG208
star: Align the icon with message content.
b17aaa6 
By wrapping the icon with a dummy Text widget in a Row, the Text helps
us stretch the row to the same height as a line of a message content
would have. This allows us to position the icon with respect to text,
even if a scaling factor has been applied.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 4, 2024
@PIG208
ui: Add edited/moved marker.
b41efe9 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 4, 2024
@PIG208
ui: Add edited/moved marker.
1961b6d 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 4, 2024
@PIG208
ui: Add edited/moved marker.
fb65a29 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 4, 2024
@PIG208
ui: Add edited/moved marker.
602fe9b 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 4, 2024
@PIG208
ui: Add edited/moved marker.
d233bdc 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 5, 2024
@PIG208
ui: Add edited/moved marker.
411e495 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 5, 2024
@PIG208
ui: Add edited/moved marker.
693c9e8 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 5, 2024
@PIG208
ui: Add edited/moved marker.
5c1c046 
This adds basic support to displaying a edited/moved marker next to the
message content. As of now this is implemented without the swipe gesture
control that would expand the marker.

Fixes zulip#616.

Partially addresses zulip#171.

Signed-off-by: Zixuan James Li <[email protected]>
@chrisbobbe chrisbobbe mentioned this issue Jul 5, 2024
Merged
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 5, 2024
@PIG208
msglist: Fix star icon alignment issue.
a48637e 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 6, 2024
@PIG208
msglist: Fix star icon alignment issue.
f6bbdc8 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 13, 2024
@PIG208
msglist: Fix star icon alignment issue.
bbadf28 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 15, 2024
@PIG208
msglist: Fix star icon alignment issue.
ac13031 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 15, 2024
@PIG208
msglist: Fix star icon alignment issue.
6860a8b 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 15, 2024
@PIG208
msglist: Fix star icon alignment issue.
d954b9b 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 16, 2024
@PIG208
msglist: Fix star icon alignment issue.
7aee822 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 16, 2024
@PIG208
msglist: Fix star icon alignment issue.
172c5d3 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

Signed-off-by: Zixuan James Li <[email protected]>
PIG208 added a commit to PIG208/zulip-flutter that referenced this issue Jul 16, 2024
@PIG208
msglist: Fix star icon alignment issue.
c2fcfc9 
By aligning the row using `textBaseline`, we no longer need to apply
the padding to the star.

Fixes zulip#616.

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
Assignees
No one assigned
Labels
a-design Visual and UX design a-msglist The message-list screen, except what's label:a-content beta feedback Things beta users have specifically asked for
Projects
Flutter app
Status: Done
Milestone
M3: Summer 2024
2 participants
@gnprice @PIG208