zulip-flutter icon indicating copy to clipboard operation
zulip-flutter copied to clipboard

compose_box: Replace compose box with a banner when cannot post in a channel

Open sm-sayedi opened this issue 1 year ago • 4 comments

When a user cannot post in a channel based on ZulipStream.channelPostPolicy, all parts of the compose box (in both channel and topic narrow) are replaced with a banner, saying: You do not have permission to post in this channel.

Screenshot

Screen recording

https://github.com/user-attachments/assets/c599db3c-755f-41b6-b119-0d6fc931f701

Fixes: #674

sm-sayedi avatar Aug 12 '24 19:08 sm-sayedi

Thanks @chrisbobbe for the review! Revision pushed with the tests cleaned up. PTAL!

sm-sayedi avatar Aug 19 '24 19:08 sm-sayedi

Thanks! Ah, it looks like this has gathered some conflicts—would you mind rebasing and resolving those please?

chrisbobbe avatar Aug 21 '24 21:08 chrisbobbe

Conflicts resolved @chrisbobbe! Please have a look!

sm-sayedi avatar Aug 22 '24 03:08 sm-sayedi

Thanks @chrisbobbe for the review. Pushed the new changes. Also added #886 comment. PTAL.

cc @PIG208

sm-sayedi avatar Oct 07 '24 03:10 sm-sayedi

Thanks @PIG208 for the review. Changes pushed. PTAL.

Also added a few comments in the thread above.

sm-sayedi avatar Oct 19 '24 08:10 sm-sayedi

Thanks @gnprice. Changes pushed.

sm-sayedi avatar Oct 21 '24 05:10 sm-sayedi

Read through the changes and left some comments mostly for style and formatting. Thanks for the revision! The test changes look clean now; I noticed some changes that probably belongs to compose_box: Replace compose box with a banner when cannot post in a channel.

PIG208 avatar Oct 24 '24 01:10 PIG208

Thanks @PIG208 for the review. Changes pushed.

sm-sayedi avatar Oct 24 '24 19:10 sm-sayedi

Looks good. Thanks! Moving this to Greg's review.

PIG208 avatar Oct 24 '24 23:10 PIG208

Thanks @gnprice for the previous review. New changes pushed. PTAL.

Also added a few comments just above and one in the previous review sub-thread.

sm-sayedi avatar Nov 12 '24 07:11 sm-sayedi

Thanks @gnprice for the review. New changes pushed. Please have a look.

sm-sayedi avatar Nov 15 '24 05:11 sm-sayedi

Thanks! Looks good — merging.

One commit-message tweak: I noticed the commits had two variants of what the prefix should be for compose-box changes: 388b83b2d compose test [nfc]: Add a parent test group f14e32dac compose_box: Replace compose box with a banner when cannot post in a channel

namely "compose" vs "compose_box". So I looked at past examples; the shorter "compose" form is what we used originally, and have still used many more times than "compose_box". So edited to "compose": 157418c26 compose: Replace compose box with a banner when cannot post in a channel


FTR here's the command I ended up using to see past usage: git log --format='%>(20)%an %cs %h %<(26,trunc)%s' --grep compose origin | grep -P '\bcompose(.box)?|'

It's a bit more sophisticated than the quick git log --oneline --grep foo commands I've used many times.

gnprice avatar Nov 15 '24 20:11 gnprice