general chat #2: support sending to empty topic from channel narrow, hide resolve/unresolve conditionally #1364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Feb 20, 2025
PIG208
force-pushed
the
pr-general-chat-2
branch
from
5382b42 to
d2a820f
Compare
PIG208
force-pushed
the
pr-general-chat-2
branch
from
d2a820f to
61d5a9c
Compare
PIG208
changed the title
PIG208
force-pushed
the
pr-general-chat-2
branch
from
61d5a9c to
5bfdaaa
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Signed-off-by: Zixuan James Li <[email protected]>
instead of reading TopicName.displayName Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
This helps imply why we don't have the check when displayName is null Signed-off-by: Zixuan James Li <[email protected]>
…ears Signed-off-by: Zixuan James Li <[email protected]>
…ppears Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
Signed-off-by: Zixuan James Li <[email protected]>
PIG208
force-pushed
the
pr-general-chat-2
branch
from
5bfdaaa to
dab6fce
Compare
A test is skipped because the server does not send empty topics to the client without "empty_topic_name" client capability. Signed-off-by: Zixuan James Li <[email protected]>
PIG208
force-pushed
the
pr-general-chat-2
branch
from
dab6fce to
2e7db9c
Compare
PIG208
force-pushed
the
pr-general-chat-2
branch
from
2e7db9c to
8c3db69
Compare
This helper will become more relevant within the context of `ComposeTopicController`. Signed-off-by: Zixuan James Li <[email protected]>
Previously, "Message #stream > (no topic)" would appear as the hint text when the topic input is empty but mandatory. The control flow of `getDestinationString` can be simplified. However, it is structured this way to prepare for a later change to support showing "general chat" in the hint text, that adds some more advanced checks. Signed-off-by: Zixuan James Li <[email protected]>
This does not rely on TopicName.displayName being non-nullable or "empty_topic_name" client capability, so it is not an NFC change. The key change that allows sending to empty topic is that `textNormalized` can now be actually empty, instead of being converted to "(no topic)" with `_computeTextNormalized`. --- This is accompanied with a content input hint text change, so that either "Message #stream > (no topic)" or "Message #stream > general chat" appears appropriately. Previously, "Message #stream > (no topic)" was the hint text for content input as long as the topic input is empty and topics are not mandatory. Showing the default topic does not help create incentive for the user to pick a topic first. So only show it when they intend to leave the topic empty. See discussion: https://chat.zulip.org/#narrow/channel/530-mobile-design/topic/general.20chat.20design.20.23F1297/near/2088870 --- This does not aim to implement hint text changes to the topic input, which is always "Topic". We will handle that as a follow-up. Signed-off-by: Zixuan James Li <[email protected]>
PIG208
force-pushed
the
pr-general-chat-2
branch
from
8c3db69 to
5f978b6
Compare
Signed-off-by: Zixuan James Li <[email protected]>
Look for `allow_empty_topic_name` and `empty_topic_name` under "Feature level 334" in the API Changelog to verify the affected routes: https://zulip.com/api/changelog To keep the API bindings thin, instead of setting `allow_empty_topic_name` for the callers, we require the callers to pass the appropriate values instead. Instead of making this parameter a `bool` that defaults to `false` (and have the bindings remove the field when it's `false`), we type it as `bool?` and only drop it when it is `null`. This is also for making the API binding thin. Fixes: zulip#1250 Signed-off-by: Zixuan James Li <[email protected]>
PIG208
force-pushed
the
pr-general-chat-2
branch
from
5f978b6 to
bb42d03
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Similar to #1297, this PR focuses on part of the general chat feature, with some final "do not merge" commits to make it testable.
This includes the following commits:
The remaining commits should be reviewed in #1297, but neither PR blocks each other, as they each focuses on a specific part of the feature.
Screenshots