Skip to content

Conversation

@WesleyAC
Copy link
Contributor

@WesleyAC WesleyAC commented Apr 7, 2021

This is the same thing that the (i) menu on the stream title currently
displays. Since we plan to remove that icon soon, allowing people to
access it here makes sense.

@WesleyAC WesleyAC requested review from chrisbobbe and gnprice April 7, 2021 10:20
@WesleyAC WesleyAC force-pushed the stream-settings-actionsheet-option branch from ea16527 to 62c3424 Compare April 7, 2021 10:21
This is the same thing that the (i) menu on the stream title currently
displays. Since we plan to remove that icon soon, allowing people to
access it here makes sense.
@gnprice gnprice force-pushed the stream-settings-actionsheet-option branch from 62c3424 to 5955c54 Compare April 7, 2021 23:36
@gnprice gnprice merged commit 5955c54 into zulip:master Apr 7, 2021
@gnprice
Copy link
Member

gnprice commented Apr 7, 2021

LGTM, merged, thanks!

@chrisbobbe
Copy link
Contributor

chrisbobbe commented Apr 7, 2021

Ah, the one comment I was about to add was that it'd be good to add these new user-facing strings to static/translations/messages_en.json. (I believe we call our _ function on them somewhere in the plumbing.)

@gnprice
Copy link
Member

gnprice commented Apr 7, 2021

Ah good catch, thanks! Want to push a quick change to do that?

And then we should perhaps do something to make that easier to notice when writing the change in the first place. A block comment about it, at the top or bottom of the list of these options, might be a good step.

@chrisbobbe
Copy link
Contributor

Sure, just sent #4619!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants