Skip to content

Conversation

@aryanranderiya
Copy link

@aryanranderiya aryanranderiya commented Mar 15, 2025

Fix leave warning modal translation interpolation

Proposed changes (including videos or screenshots)

This pull request addresses an issue in the leave room warning modal where the translation string wasn’t interpolating the room name correctly, resulting in the %s placeholder being displayed instead of the actual room name. The changes include:

  • Updating the translation hook import: Now using useTranslation from @rocket.chat/ui-contexts instead of react-i18next
  • Correcting the interpolation format: Adjusting the way the room name is passed to the translation function so that it correctly replaces the %s placeholder.

Before:

2025-03-16_02-12

After:

2025-03-16_02-14

Fixes #35289


Steps to test or reproduce

  1. Open a room in Rocket.Chat.
  2. Trigger the leave room action (e.g., click the leave button)
  3. Verify that the warning modal displays the actual room name instead of %s.

Further comments

This fix ensures that the leave room warning modal now provides clear, user-friendly feedback by correctly interpolating the room name. Please review and let me know if further adjustments or additional tests are required.

This issue was particularly difficult to debug because I tested multiple formatting variations, but none of them worked. The main culprit turned out to be a sneaky different import of useTranslation, which caused unexpected behavior. Fixing this import, and changing the formatting resolved the issue.

A similar fix was attempted in #35290 . In that PR, gabriellsh requested changes with the comment:

You have to correct the translation string. String interpolation shouldn't be handled manually

Originally posted by @gabriellsh in #35290 (review)

Please review and let me know if further adjustments or additional tests are required.

@aryanranderiya aryanranderiya requested a review from a team as a code owner March 15, 2025 20:54
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Mar 15, 2025

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Mar 15, 2025

⚠️ No Changeset found

Latest commit: 005a87e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@Ansh-dS
Copy link
Contributor

Ansh-dS commented Mar 16, 2025

I have updated the code now. #35290

@aryanranderiya
Copy link
Author

I have updated the code now. #35290

There is no need, I had already pushed the fix.

@Ansh-dS
Copy link
Contributor

Ansh-dS commented Mar 16, 2025

you have wasted your time but I didn't said this thing. check the new messages I have posted #35290.

@gabriellsh
Copy link
Member

Instead of focusing your energy in disputing who will fix what, you could try to help each other improve your fixes and be cordial to each other.

Both your PR's have several issues that need to be addressed, and since you'd rather fight instead of reading the documentation and following the rules to open a proper PR, I fixed this issue here: #35568

I'll be closing both PRs. Take this as a warning. We don't want this kind of behaviour, and it paints a terrible picture for you guys. This is not a competition.

@gabriellsh gabriellsh closed this Mar 20, 2025
@Ansh-dS
Copy link
Contributor

Ansh-dS commented Mar 21, 2025

New learning for me:
I made this mistake but next time whenever I stuck into these situations I will try to collaborate not compete. Thanks.

@gabriellsh
Copy link
Member

Great to hear! Let's collaborate and help each other out.

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.

fix: Channel Name Not Displayed in Leave Confirmation Dialog

3 participants