messages: Adding prefix symbols to search bar. #1441
Conversation
There was a problem hiding this comment.
@supascooopa Thanks for the followup 👍
This looks to work well and cover most narrows, except the DM subsets I refer to inline - did you intend to not cover these?
The leading space looks good, but I think it would be useful to borrow a space from that currently used to separate the narrow text from the Search [/]: (currently 2?). We can color it per the narrow to make it look a little more balanced, less abrupt after the end of the narrow name, and more similar to the spacing after the stream-topic separator. That will involve touching a different part of the code, so I'd suggest that be a separate commit after (or before) the current one.
zulipterminal/ui_tools/messages.py
Outdated
| (bar_color, self.stream_name), | ||
| (bar_color, ": topic narrow"), | ||
| ], | ||
| ) | ||
| else: | ||
| text_to_fill = ( | ||
| "bar", # type: ignore[assignment] | ||
| [(bar_color, self.stream_name)], | ||
| [(bar_color, f" {stream_icon} "), (bar_color, self.stream_name)], | ||
| ) | ||
| elif len(curr_narrow) == 1 and len(curr_narrow[0][1].split(",")) > 1: | ||
| text_to_fill = "Group direct message conversation" |
There was a problem hiding this comment.
I noticed this wasn't updated during testing? I know we discussed different symbols, but for now I think we should have a symbol for all the narrows we support - otherwise it looks inconsistent.
There was a problem hiding this comment.
I'm not %100 sure what you mean. Would you please elaborate more? If you mean using "#" sign for the search bar part of the testing, I could add a different symbol to represent all the narrows. Do you have any suggestions?
There was a problem hiding this comment.
Is this what you meant by the testing update you mentioned in the stream? This comment was regarding how the 'Group direct message conversation' has no prefix, as well as one other DM-related one also.
There was a problem hiding this comment.
I got confused because of the changed code highlights. I thought you were referring to the changes I made that were highlighted and not updating tests accordingly. I thought I was missing something, but I understand now.
Like I mentioned in the stream, I was just unclear about which symbol we landed on last regarding the DMs, but I'm sure now and I'll make the necessary additions soon.
|
@neiljp Thank you for the feedback! I will look into all the suggestions you've made. |
neiljp
added
the
PR awaiting update
supascooopa
force-pushed
the
adding_prefix_symbols
branch
from
ef997fa
to
7413507
Compare
As previously done to other parts of the UI, this commit adds prefix symbols to the narrow title (before the message search input area). This is applied to all supported narrows: - all messages - all direct messages - mentions - starred messages - single stream - single topic - direct message conversations (1:1 and group) The spacing around the prefixes is laid out to be similar to that for message headings, so that the icons line up cleanly. This is intended to make the UI more similar to the web app UI and unify the overall look. Tests adjusted accordingly.
The colored background area of stream message headings are already visually balanced with a leading and trailing space. The narrow title prefixes added in the previous commit contain a leading space to align the prefixes with those in message headings; this commit adds a trailing space, which balances the colored area in a similar way to the stream message headings and avoids an abrupt end to the narrow text. The existing spacing between the narrow title and message search text is maintained by removing one of the two spaces between the `Search [/]:` and the narrow title with a general background color, and substituting it with an additional space at the end of each narrow title. Tests adjusted accordingly.
neiljp
force-pushed
the
adding_prefix_symbols
branch
from
d5a6b31
to
b1b7caf
Compare
neiljp
added
PR ready to be merged
|
@supascooopa Thanks for this - it's looking good! 🎉 As per my note in the stream, I squashed a couple of commits and adjusted the messages to fit our style and expand upon the intention. Note how I using 'Show' instead of 'Showing'. |
As previously done to other parts of the UI, this commit adds prefix symbols to the narrow bar before the search input area.
This is intended to make the UI more similar to the web app UI and unify the overall look.
This is applied to both streams, topics and main narrows.
Tests adjusted accordingly.
What does this PR do, and why?
This PR adds prefix symbols to the narrow titles before the search bar. This is done in order to approximate the current UI to the web app and to unify the overall look of the UI.
External discussion & connections
Narrow Symbols #T1420How did you test this?
Self-review checklist for each commit
Visual changes
Before:
After:
