Skip to content

Conversation

@Ansh-dS
Copy link
Contributor

@Ansh-dS Ansh-dS commented Feb 21, 2025

Proposed changes (including videos or screenshots)

The %s placeholder is getting replaced with the actual group/channel name in the warning message.

Screen.Recording.2025-02-21.123100.mp4

Issue(s)

#35289

Steps to test or reproduce

  1. Create a Group, Channel, or Team.
  2. Locate the newly created group/channel in the left pane under Omnichannel.
  3. Click on the three-dot menu next to the group/channel.
  4. Select "Leave" from the dropdown.

@Ansh-dS Ansh-dS requested a review from a team as a code owner February 21, 2025 07:48
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Feb 21, 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

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Ansh-dS seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Ansh-dS Ansh-dS changed the title Channel Name Not Displayed in Leave Confirmation Dialog Bug: Channel Name Not Displayed in Leave Confirmation Dialog Feb 21, 2025
@Ansh-dS Ansh-dS changed the title Bug: Channel Name Not Displayed in Leave Confirmation Dialog Fix: Channel Name Not Displayed in Leave Confirmation Dialog Feb 21, 2025
@Ansh-dS Ansh-dS changed the title Fix: Channel Name Not Displayed in Leave Confirmation Dialog fix: Channel Name Not Displayed in Leave Confirmation Dialog Feb 21, 2025
Copy link
Member

@gabriellsh gabriellsh left a comment

Choose a reason for hiding this comment

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

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

@Ansh-dS
Copy link
Contributor Author

Ansh-dS commented Feb 28, 2025

There are two files that contain translations named "packages\i18n\locales\en.i18n.json" andpackages\i18n\locales\se.i18n.json," and these files contain values.

There is another file that produces the same warning while, with room.fname and room.name as differences, containing the following code but correctly converting "are you sure you want to leave...":

	const warnText = roomCoordinator.getRoomDirectives(room.t).getUiText(UiTextContext.LEAVE_WARNING);
	setModal(
<Warning
={( as TranslationKey room.fname || room.name)}
);

After this, I checked the type of "name" and the type of "room.name" or "room.fname," and all are strings.

I also used curly braces around "name," doubting whether the translation is catching it or not, but in the end, the values.

@aryanranderiya
Copy link

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

I have fixed this issue here: #35522 @Ansh-dS @gabriellsh

@changeset-bot
Copy link

changeset-bot bot commented Mar 16, 2025

⚠️ No Changeset found

Latest commit: 1a98f9d

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 Author

Ansh-dS commented Mar 16, 2025

@aryanranderiya, please don't mention mentors. I have been working on my new PR and haven't time to work this recently. You also didn't ask for confirmation. Besides, I reviewed your code and noticed that you have added many unusual lines butThe only change needed is to update the import for useTranslation.But now I have updated the code Thanks.

@aryanranderiya
Copy link

aryanranderiya commented Mar 16, 2025

@aryanranderiya, please don't mention mentors. I have been working on my new PR and haven't time to work this recently. You also didn't ask for confirmation. Besides, I reviewed your code and noticed that you have added many unusual lines butThe only change needed is to update the import for useTranslation.But now I have updated the code Thanks.

What do you mean unusual lines? I have just re-ordered / refactored a few lines, there are no other functional changes. Also there was no need to add a new commit when I have already pushed the same change? I spent a long time debugging the fix and you have literally copied the code

@aryanranderiya
Copy link

@aryanranderiya, please don't mention mentors. I have been working on my new PR and haven't time to work this recently. You also didn't ask for confirmation. Besides, I reviewed your code and noticed that you have added many unusual lines butThe only change needed is to update the import for useTranslation.But now I have updated the code Thanks.

What do you mean ask for confirmation?

@Ansh-dS
Copy link
Contributor Author

Ansh-dS commented Mar 16, 2025

@aryanranderiya that you didn't ask weather "i should work or not"(it's a common practice before started working many persons make this and ended up getting rejected) . since I want to work on this but later and why I want to work latter that I have already mentioned

@Ansh-dS
Copy link
Contributor Author

Ansh-dS commented Mar 16, 2025

@anandpathak

the process to work on any issue is:

  1. Ask whether somebody is working on it or not.( common practice in github, otherwise you end up making chaos or wasting not only yours but also others effort. )

As far as work concerned:

  1. I have documented and found this issue, then I tested whether it's a real issue or not.
  2. I figured out the files.
  3. Then I debugged ensure there is no problem with the "translation string" (as in the above comments by the mentor).
  4. As this issue was time, I decided to work it afterwards (that's why one should first about to work on or not).

As far your work:

You have a great job, but if I or add my efforts with yours, you have done less than half of the efforts I have put in (as mentioned). I have created things from scratch, only this but many. If you don't make your PR, I will eventually figure out the main problem. isn't a big deal for me, as if I can make 7-8 PRs then solving this is not a big deal, I just delayed for a while and you didn't ask anything.

@gabriellsh
Copy link
Member

Closing this, read this comment #35522 (comment)

@gabriellsh gabriellsh closed this Mar 20, 2025
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.

5 participants