-
Notifications
You must be signed in to change notification settings - Fork 397
compose: Show the Refresh/Subscribe banner when can send, too #2084
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
base: main
Are you sure you want to change the base?
compose: Show the Refresh/Subscribe banner when can send, too #2084
Conversation
The message is "New messages will not appear automatically." and the buttons are "Refresh" and "Subscribe".
We'll keep using this for the case where you don't have permission to send messages in the channel. It's not quite appropriate for the case of an *unknown* channel, though, and we'll start showing something different for that, coming up.
As suggested by Alya: zulip#1947 (comment)
Soon we'll stop offering the compose box when one or more of the DM recipients are unknown. Fix these tests to not fail with that change, by adding appropriate user data to the store.
Fixes zulip#1798. In zulip#1873, we started showing a Refresh/Subscribe banner when viewing an unsubscribed channel -- but only if you didn't have permission to send messages there. We postponed showing it in the case where you *can* send messages because the UI message for that case is "Replies to your messages will not appear automatically", and that message would have been confusing in a world where your own messages didn't appear automatically either because of bugs that hadn't been fixed. We've fixed those bugs in zulip#2034 -- the "first" and "third" parts of zulip#1798 -- so now go ahead and offer the Refresh/Subscribe banner, with the planned text, in the case where you can send messages. This completes the work for zulip#1798.
rajveermalviya
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chrisbobbe! All LGTM and tests great. Couple of comments below about lingering l10n entries, I am not sure if Weblate automatically garbage collects them.
Moving over to Greg's review.
| "composeBoxBannerLabelCannotSendInChannel": "You do not have permission to post in this channel.", | ||
| "@composeBoxBannerLabelCannotSendInChannel": { | ||
| "description": "Label text for a banner replacing the compose box when you do not have permission to send messages in the channel." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit 7f397e42a413544a6830ea5c7f567630223a0fac
compose [nfc]: Rename a localizations string for its intended use
…
I think you'll also need to manually remove errorBannerCannotPostInChannelLabel and @errorBannerCannotPostInChannelLabel entries from other assets/l10n/app_*.arb files. Not sure if there is better way to do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better yet, rename those entries. That way Weblate will pick up the existing values, and won't ask translators to re-translate the strings.
I'd do that with a find-and-replace: from the command line, something like perl -i -pe 's/errorBannerCannotPostInChannelLabel/composeBoxBannerLabelCannotSendInChannel/g' assets/l10n/*.arb, but if one has a favorite other find-and-replace tool then that should work fine too.
| "composeBoxBannerLabelDeactivatedDmRecipient": "You cannot send messages to deactivated users.", | ||
| "@composeBoxBannerLabelDeactivatedDmRecipient": { | ||
| "description": "Label text for a banner replacing the compose box when you cannot send messages in the DM conversation because one or more members are deactivated." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commit 92cff0af1cc87fd91e3f9dffc5d35d1910f4a734
compose [nfc]: Name/describe a localization string more like its friends
Same here for errorBannerDeactivatedDmLabel and @errorBannerDeactivatedDmLabel.
|
Thanks @chrisbobbe for taking care of this, and @rajveermalviya for the previous review! The code all LGTM except the points Rajesh mentioned above. If @alya has product feedback, we can also incorporate that, though this situation in the app isn't a critical one so it's possible she won't get to it soon. |










Fixes #1798.
Notable commit message:
Also a few other behavior changes:
Screenshots coming up.