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

info_density: Calculate values for vertical alignment and apply to emoji. #30050

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

karlstolley
Copy link
Contributor

@karlstolley karlstolley commented May 10, 2024

This PR calculates values necessary to properly fit and vertically align inline-block elements within any given line height and font size combination. Those values are then applied as an example to inline emoji, though the techniques and values here are strong candidates to be extended to other blocks appearing within the message area (e.g., todo widget checkboxes, <time> elements).

Additionally, previous fiddly top and margin values applied to emoji have been removed here, so that similarly fiddly adjustments are made unnecessary as users adjust information density values. A commit added on 11 June 2024 now also restores the oversize-emoji look that Zulip is known for, but critically without disturbing the surrounding lines of text (updated screenshots below).

CZO discussion

To test this PR in dev, you'll see the default values just by running dev on this branch. The database-stored legacy font size and line-height equivalents can be made active by deselecting Dense mode under Settings > Preferences. To see the 16px/140% version, set those values in the Message-area font size (px) and Message-area line height (%). Same with any other reasonable combination of font size and line-height you might be curious to examine.

Screenshots and screen captures:

Before After (oversize, legacy .more_dense_mode)
inline-emoji-main-2024-06-11 inline-emoji-oversize-legacy

The screenshots here demonstrate that the oversize versions do not disturb the flow of text compared to their line-fitted counterparts.

Legacy, line-fitted Legacy, oversize
inline-emoji-line-fitted-legacy inline-emoji-oversize-legacy
14/122, line-fitted 14/122, oversize
inline-emoji-line-fitted-14-122 inline-emoji-oversize-14-122

The change at 16/140 is far less dramatic because the line-fitted size comes much closer to the oversize size.

16/140, line-fitted 16/140, oversize
inline-emoji-oversize-16-140 inline-emoji-line-fitted-16-140
Outdated screenshots | Before | After (legacy font size/line height values) | | --- | --- | | ![legacy-emoji-before](https://github.com/zulip/zulip/assets/170719/6a35ad2a-ddf2-417e-b322-0dcc87e0814b) | ![legacy-emoji-after-calculated-offset](https://github.com/zulip/zulip/assets/170719/af241f8a-4d00-49bc-a9fa-0ac47ab531a5) |
16px/140% 100px/100% (exaggerated, line-fitted) 100px/300% (exaggerated, font-size-clamped)
emoji-16-140 emoji-100-100 emoji-100-300

Showing shift-free text at 16px/140%

16-140-shifts

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@karlstolley karlstolley added area: emoji Emoji in markup, emoji reactions, emoji picker, etc. chat.zulip.org review Added by maintainers when a PR requires chat.zulip.org testing to proceed. labels May 10, 2024
@zulipbot
Copy link
Member

Hello @zulip/server-emoji members, this pull request was labeled with the "area: emoji" label, so you may want to check it out!

@timabbott timabbott added the deployed on chat.zulip.org Added by maintainers when a PR is currently being tested on chat.zulip.org. label May 14, 2024
@timabbott
Copy link
Sponsor Member

Test-deploying on chat.zulip.org.

@karlstolley karlstolley force-pushed the em-fitted-inline-emoji branch 4 times, most recently from b76960b to 5e3b904 Compare May 16, 2024 16:52
@zulipbot zulipbot added size: XL and removed size: L labels May 16, 2024
@karlstolley
Copy link
Contributor Author

Okay. I have updated this with an entirely different approach, as discussed on CZO--which discussion I'll be updating shortly with some of the things I've discovered here and want to share more widely.

@karlstolley karlstolley force-pushed the em-fitted-inline-emoji branch 2 times, most recently from f932bbc to 2df3380 Compare May 21, 2024 21:09
@timabbott
Copy link
Sponsor Member

Updating what's deployed on chat.zulip.org.

@karlstolley
Copy link
Contributor Author

Updated to restore legacy oversize emoji. CZO discussion

@timabbott timabbott merged commit db5f584 into zulip:main Jun 11, 2024
7 checks passed
@timabbott
Copy link
Sponsor Member

Merged, thanks @karlstolley!

karlstolley added a commit to karlstolley/zulip that referenced this pull request Jun 18, 2024
Owing to logic added in zulip#30050, which accounts for the legacy line-
height value, toggling dense mode requires recalculating the
typography vars--otherwise, a non-legacy line-height value will
not be picked up until a refresh.
timabbott pushed a commit that referenced this pull request Jun 18, 2024
Owing to logic added in #30050, which accounts for the legacy line-
height value, toggling dense mode requires recalculating the
typography vars--otherwise, a non-legacy line-height value will
not be picked up until a refresh.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: emoji Emoji in markup, emoji reactions, emoji picker, etc. chat.zulip.org review Added by maintainers when a PR requires chat.zulip.org testing to proceed. deployed on chat.zulip.org Added by maintainers when a PR is currently being tested on chat.zulip.org. size: XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants