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

Display of starred and unstarred messages enhanced #1147

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Dishti-Oberai
Copy link
Collaborator

@Dishti-Oberai Dishti-Oberai commented Jan 6, 2022

What does this PR do?
This PR enhances the display of consecutive starred and/or unstarred messages which have identical dates, times, and authors.
Earlier all such messages which had similar star status (whether starred or unstarred) appeared together and messages which had different star status were separated by a line. However, this led to difficulty in identifying the star status of messages (which were starred and thus appeared together). The following screenshot depicts the same-
image

Now, all starred messages and messages which have different star statuses have been separated by a blank line whereas those which are all unstarred appear together. This leads to a better visual display and interpretation of the star status of messages.
The following screenshot demonstrates the present scenario-
image

(The first two messages are starred and the third one is unstarred in both the above screenshots)

Fixes #171

A detailed explanation of the issue and discussion related to it can be read at - https://chat.zulip.org/#narrow/stream/206-zulip-terminal/topic/Message.20merging.20-.20star.20status.20.28issue.20.23T171.29

Tested?

  • Manually
  • Existing tests (adapted, if necessary)

Notes & Questions
Should any other changes be made?

Interactions
The changes have been made (developed on) in addition to the following PR - #335

Visual changes
Earlier-
image
image
image

Presently-
image
image
image
image

@zulipbot zulipbot added the size: XS [Automatic label added by zulipbot] label Jan 6, 2022
@zulipbot
Copy link
Member

zulipbot commented Jan 6, 2022

Hello @Dishti-Oberai, it seems like you have referenced #171 in your pull request description, but you have not referenced them in your commit message description(s). When you reference an issue in a commit message, it automatically closes the corresponding issue when the commit is merged.

Please run git commit --amend in your command line client to amend your commit message description with Fixes #171..

An example of a correctly-formatted commit:

commit fabd5e450374c8dde65ec35f02140383940fe146
Author: zulipbot
Date:   Sat Mar 18 13:42:40 2017 -0700

    pull requests: Check PR commits reference when issue is referenced.

    Fixes #51.

Thank you for your contributions to Zulip!

@zulipbot zulipbot added size: L [Automatic label added by zulipbot] size: XS [Automatic label added by zulipbot] and removed size: XS [Automatic label added by zulipbot] size: L [Automatic label added by zulipbot] labels Jan 13, 2022
@zulipbot zulipbot added size: S [Automatic label added by zulipbot] size: XS [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] size: S [Automatic label added by zulipbot] labels Jan 25, 2022
@neiljp neiljp added the area: UI General user interface update label Jan 30, 2022
@zulipbot zulipbot added size: S [Automatic label added by zulipbot] and removed size: XS [Automatic label added by zulipbot] labels Feb 2, 2022
@zulipbot zulipbot added size: XS [Automatic label added by zulipbot] and removed size: S [Automatic label added by zulipbot] labels Feb 9, 2022
@Dishti-Oberai Dishti-Oberai force-pushed the StarStatus branch 6 times, most recently from 55f9818 to 24f41cf Compare February 9, 2022 03:12
@Dishti-Oberai Dishti-Oberai force-pushed the StarStatus branch 5 times, most recently from 1eb3c72 to 8207fae Compare February 19, 2022 08:27
@neiljp neiljp added the PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback label Mar 21, 2022
@neiljp
Copy link
Collaborator

neiljp commented Mar 22, 2022

@Dishti-Oberai I've marked this as awaiting update, since I assume you're still working on tests.

@Dishti-Oberai
Copy link
Collaborator Author

Thanks for reminding me regarding this @neiljp !
I am working on it presently but it might take some time as I have exams upcoming at my institute too :)
Will try my level best to complete this at the earliest.

@zulipbot zulipbot added size: XL [Automatic label added by zulipbot] and removed size: XS [Automatic label added by zulipbot] labels Mar 27, 2022
@Dishti-Oberai Dishti-Oberai force-pushed the StarStatus branch 2 times, most recently from a484b3e to b7f894b Compare March 27, 2022 01:09
@Dishti-Oberai
Copy link
Collaborator Author

@zulipbot add "PR needs review"

@zulipbot zulipbot added the PR needs review PR requires feedback to proceed label Mar 27, 2022
@Dishti-Oberai Dishti-Oberai force-pushed the StarStatus branch 3 times, most recently from 9c0b3b2 to 711d937 Compare March 27, 2022 07:23
@zulipbot zulipbot added size: M [Automatic label added by zulipbot] and removed size: XL [Automatic label added by zulipbot] labels Mar 27, 2022
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.

@Dishti-Oberai I left comments in the stream.

The test changes so far make the tests pass, but they don't yet cover the behavior, which is why I can change the new logic and still get the tests to pass.

@neiljp neiljp removed the PR needs review PR requires feedback to proceed label Apr 2, 2022
@zulipbot
Copy link
Member

Heads up @Dishti-Oberai, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

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 has conflicts PR awaiting update PR has been reviewed & is awaiting update or response to reviewer feedback size: M [Automatic label added by zulipbot]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix issues with message merging (hiding content headers) based on star status
3 participants