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

popup: core: Use linear scaling for small terminal sizes. #1018

Merged
merged 1 commit into from
May 2, 2021

Conversation

Rohitth007
Copy link
Collaborator

@Rohitth007 Rohitth007 commented Apr 30, 2021

This commit is a step towards solving #1005 especially the first
2 points. This changes the fixed scaling fraction of 3/4 to one that
is linear in the range 80 - 100 column width:

  • The scaling fraction increases from 3/4 to 1 (full-width) as the
    available size increases from 80 to 100. This increases the range of
    sizes for which the popup is rendered properly.
  • For width's smaller than 80 it defaults to full width.
  • For width's greater than 100 it defaults to 3/4.

The values 80 and 100 are given by:

  • MIN_SUPPORTED_POPUP_WIDTH = 80
  • MAX_LINEAR_SCALING_WIDTH = 100

The popup height has a fixed scaling factor of 2/3.

Tests added

80 < Width < 100

Width = 80

Width > 100 (Full screen)

@zulipbot zulipbot added the size: M [Automatic label added by zulipbot] label Apr 30, 2021
@Rohitth007
Copy link
Collaborator Author

@zulipbot add "PR needs review"

@zulipbot zulipbot added the PR needs review PR requires feedback to proceed label Apr 30, 2021
Copy link
Collaborator

@neiljp neiljp left a comment

Choose a reason for hiding this comment

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

@Rohitth007 See my feedback in the stream, but I'm essentially interested in whether we should scale the height based on the width, or treat it differently based on another rule.

@neiljp neiljp added feedback wanted and removed PR needs review PR requires feedback to proceed labels May 1, 2021
@Rohitth007 Rohitth007 force-pushed the popup-scale branch 4 times, most recently from bddcdbf to c3dc4e8 Compare May 2, 2021 08:59
This commit is a contribution towards zulip#1005, allowing popups such as the
help menu to show properly on an 80-column width terminal.

This changes the fixed scaling factor of 3/4 to one that is linear in
the range 80 - 100 column width:
* The scaling fraction increases from 3/4 to 1 (full-width) as the
available size increases from 80 to 100. This increases the range of
sizes for which the popup is rendered properly.
* For width's smaller than 80 it defaults to full width.
* For width's greater than 100 it defaults to 3/4.

The values 80 and 100 are given by:
* MIN_SUPPORTED_POPUP_WIDTH = 80
* MAX_LINEAR_SCALING_WIDTH = 100

The popup height has a fixed scaling factor of 3/4, which retains
visibility of the title when the window is at the minimum supported
height.

Tests added.
@neiljp neiljp merged commit f093bc4 into zulip:main May 2, 2021
@neiljp
Copy link
Collaborator

neiljp commented May 2, 2021

@Rohitth007 Adjusted comment and expressions for clarity, and commit text slightly, and just merged 🎉
(the change should be visible here)

@neiljp neiljp added this to the Next Release milestone May 2, 2021
@Rohitth007 Rohitth007 deleted the popup-scale branch May 3, 2021 06:07
@neiljp neiljp added area: UI General user interface update enhancement New feature or request and removed feedback wanted labels Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI General user interface update enhancement New feature or request size: M [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants