Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds feature #1309 within poup.js #2718

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Parvchannel
Copy link

PR Update: Confirmation Dialog for Sorting Tabs

Hi @dannycolin

Thank you for the guidance I've implemented the suggested changes by moving the confirmation dialog inside the add-on popup. Now, everything works as expected—users will see a confirmation before sorting tabs, ensuring a consistent experience across all pages.

However, I’m facing one issue: the changes I made to messages.json in _locales @ bdaa012 are not being reflected in my PR. I’d appreciate any insights on why this might be happening and how to resolve it.

Looking forward to your feedback. Thanks!

@Parvchannel Parvchannel changed the title adds feature #1309 within poup.js as suggested adds feature #1309 within poup.js Feb 10, 2025
@dannycolin dannycolin self-requested a review February 11, 2025 16:25
@dannycolin
Copy link
Collaborator

dannycolin commented Feb 11, 2025

For the translation, you need to open a separated PR in https://github.com/mozilla-l10n/multi-account-containers-l10n/ and only add the necessary strings for the english locales. Other locales are translated on pontoon.mozilla.org.

So far, this PR looks good to me. Do not forget to npm run lint locally and fix any issue introduced by your changes. You can ignore warnings that already exist especially those related to Unsafe assignment to innerHTML.

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.

2 participants