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

Indent to distinguish resolved topics. #1124

Closed
wants to merge 1 commit into from

Conversation

Rohitth007
Copy link
Collaborator

@Rohitth007 Rohitth007 commented Aug 20, 2021

What does this PR do?
This PR indents all TopicButtons to make space for the resolved
topic symbol and easily distinguish between them.

To make topic name length the same as before, the default suffix is
made from 2 spaces to 1 space and the LEFT_WIDTH is increased by 1.
This is equivalent to no difference in topic name length and 2 char
gain in stream/special narrow buttons.

Tested?

  • Manually
  • Existing tests (adapted, if necessary)
  • New tests added (for any new behavior)
  • Passed linting & tests (each commit)

Visual changes

@zulipbot zulipbot added the size: S [Automatic label added by zulipbot] label Aug 20, 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 This looks good, though it would be simpler with a separate API-specific addition for the resolved prefix.

My only other concern is that a long list of unresolved topics looks significantly indented - maybe more so with the text flowing closer to the right hand side.

@neiljp neiljp added this to the Next Release milestone Aug 20, 2021
@neiljp neiljp added area: UI General user interface update PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Aug 20, 2021
This commit indents all TopicButtons to make space for the resolved
topic symbol and easily distinguish between them.

A RESOLVED_TOPIC symbol has been added to api_types to be used for
recognizing a resolved topic.

To make topic name length the same as before, LEFT_WIDTH is
increased by 2. This is equivalent to no difference in topic name
length and 2 char gain in stream/special narrow buttons.

Tests updated.
@zulipbot zulipbot added size: M [Automatic label added by zulipbot] and removed size: S [Automatic label added by zulipbot] labels Aug 21, 2021
(1, 14, "topic2", "GSoC"),
(1000, 205, "topic3", "PTEST"),
(2, 86, "topic1", "Django", False),
(1, 14, "✔ topic2", "GSoC", True),
Copy link
Member

Choose a reason for hiding this comment

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

Since we have a constant defined for the Unicode, let's use it here as well.

@neiljp
Copy link
Collaborator

neiljp commented Aug 26, 2021

@Rohitth007 Thanks for extracting this - this looks cleaner! 🎉 I've merged this manually and made a quick follow-up refactor to adjust the constant to match the server, which includes the space and TOPIC in the constant value and name respectively.

@neiljp neiljp closed this Aug 26, 2021
@neiljp neiljp added missing feature: user A missing feature for all users, present in another Zulip client and removed PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback labels Aug 26, 2021
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 missing feature: user A missing feature for all users, present in another Zulip client size: M [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants