Skip to content

Conversation

chrisbobbe
Copy link
Collaborator

See Alya's feedback on #1890:

#1890 (comment)

In general, our pattern is to always show a privacy marker or (if
not convenient) a # before a channel name. It can be a separate
PR, but can we add that to these confirmation dialogs?

Doing this in code, rather than in the translators' source string, because we don't want it to vary by language.

@chrisbobbe chrisbobbe added the maintainer review PR ready for review by Zulip maintainers label Oct 1, 2025
@chrisbobbe
Copy link
Collaborator Author

cc @alya

(Ignore the dialog's body text, which will change in #1887; just look at the title text.)

Before After
image image

Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @chrisbobbe! LGTM, over to Greg's review.

@rajveermalviya rajveermalviya added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Oct 3, 2025
@rajveermalviya rajveermalviya requested a review from gnprice October 3, 2025 17:16
@gnprice gnprice force-pushed the pr-hash-in-confirmation-dialog branch from ba79788 to 410250c Compare October 3, 2025 23:02
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good; one comment.

I've also pushed a rebased version (since I already did the rebase, in preparation to merge).


final dialog = showSuggestedActionDialog(context: pageContext,
title: zulipLocalizations.unsubscribeConfirmationDialogTitle(subscription.name),
title: zulipLocalizations.unsubscribeConfirmationDialogTitle('#${subscription.name}'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should get a matching change in the example for this placeholder in app_en.arb.

…itle

See Alya's feedback on zulip#1890:

zulip#1890 (comment)
> In general, our pattern is to always show a privacy marker or (if
> not convenient) a `#` before a channel name. It can be a separate
> PR, but can we add that to these confirmation dialogs?

Doing this in code, rather than in the translators' source string,
because we don't want it to vary by language.
@chrisbobbe chrisbobbe force-pushed the pr-hash-in-confirmation-dialog branch from 410250c to fd5b9a3 Compare October 3, 2025 23:52
@chrisbobbe
Copy link
Collaborator Author

Thanks! Revision pushed.

@gnprice
Copy link
Member

gnprice commented Oct 4, 2025

Thanks! Looks good; merging.

@gnprice gnprice merged commit fd5b9a3 into zulip:main Oct 4, 2025
1 check passed
@chrisbobbe chrisbobbe deleted the pr-hash-in-confirmation-dialog branch October 4, 2025 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants