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.

Sign up for GitHub

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

Jump to bottom

fix: fix token detection modal stuck on firefox #25279

Merged
merged 3 commits into from
Jun 13, 2024
Merged

fix: fix token detection modal stuck on firefox #25279

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d6e2b5
fix: fix token detection modal stuck on firefox
sahar-fehri Jun 12, 2024
0bd64cb
fix: add locale back
sahar-fehri Jun 12, 2024
43e4a1a
Merge branch 'develop' into fix/token-detection-modal-stuck
legobeat Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ui/components/app/auto-detect-token/auto-detect-token-modal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ function AutoDetectTokenModal({
const dispatch = useDispatch();
const trackEvent = useContext(MetaMetricsContext);
const { chainId } = useSelector(getProviderConfig);
const locale = useSelector(getCurrentLocale);

const handleTokenAutoDetection = useCallback(
(val) => {
Expand All @@ -55,7 +56,7 @@ function AutoDetectTokenModal({
category: MetaMetricsEventCategory.Navigation,
properties: {
chain_id: chainId,
locale: getCurrentLocale,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @salimtb 👋 any idea why this was needed in the first place?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhhh, we're trying to set a value of a function instead of the locale itself. So assuming we need the locale, we could do:

const locale = useSelector(getCurrentLocale)

Is this correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right that fixes it, i haven't seen any trackEvent passing the locale, so i removed, i guess i can put it back just in case and merge 🙏 thanks!

Copy link
Contributor

@bergeron bergeron Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think selecting the locale here is fine, although it may not be necessary since it looks like events automatically get locale added on this line.

metamask-extension/app/scripts/controllers/metametrics.js

Line 761 in 92e6b94

locale: this.locale,

locale,
referrer: ORIGIN_METAMASK,
},
});
Expand Down