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

[docs] Fix about page flags #29314

Merged
merged 1 commit into from
Oct 31, 2021
Merged

[docs] Fix about page flags #29314

merged 1 commit into from
Oct 31, 2021

Conversation

mbrookes
Copy link
Member

@mbrookes mbrookes commented Oct 26, 2021

@mbrookes mbrookes added the docs Improvements or additions to the documentation label Oct 26, 2021
@mbrookes mbrookes requested a review from siriwatknp October 26, 2021 16:11
@mui-pr-bot
Copy link

mui-pr-bot commented Oct 26, 2021

No bundle size changes

Generated by 🚫 dangerJS against dd74395

Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Nice.

@michaldudak
Copy link
Member

We could store the flags in our repo (it's just a few pngs after all) and not ever worry about another service being down again.

@mnajdova
Copy link
Member

We use different URL for the autocomplete countries, maybe we can use the same? https://github.com/mui-org/material-ui/blob/master/docs/src/pages/components/autocomplete/CountrySelect.tsx#L19

@siriwatknp
Copy link
Member

We could store the flags in our repo (it's just a few pngs after all) and not ever worry about another service being down again.

I suggest improving it later (maybe in the next quarter). @mbrookes your call.

@oliviertassinari
Copy link
Member

oliviertassinari commented Oct 30, 2021

Nice to see that we have consolidated on a single flag provider (same as https://deploy-preview-29314--material-ui.netlify.app/components/autocomplete/#country-select). The downside of this one is that the flag aren't squared, it doesn't provide the same visual output, but it's fine 👍

We use different URL for the autocomplete countries, maybe we can use the same? https://github.com/mui-org/material-ui/blob/master/docs/src/pages/components/autocomplete/CountrySelect.tsx#L19

@mnajdova I gave it a try, the png yield icons that don't look as great as the SVG (compression), the png is more efficient for the bandwidth, it's really true when the flag is complex.

We could store the flags in our repo (it's just a few pngs after all) and not ever worry about another service being down again.

I would anticipate this to be more time-consuming than changing the CDN provider every few years.

@mbrookes mbrookes merged commit 536f102 into mui:master Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants