search

Semantic-Org / Semantic-UI

Semantic is a UI component framework based around useful principles from natural language.
http://www.semantic-ui.com
MIT License
51.11k stars 4.94k forks source link

[Flag] - There are 2 hidden flags #6926

Open rubenprodev opened 4 years ago

rubenprodev commented 4 years ago

Steps to Reproduce

  1. Go to https://semantic-ui.com/elements/flag.html
  2. Go to Types and see the full flag list.
  3. If you count, you get 245 flags.

Expected We must have the same exact number of flags in the flags.png sprite, which is used for the component (https://github.com/Semantic-Org/Semantic-UI/blob/master/dist/themes/default/assets/images/flags.png)

Result Instead, we got 2 more. One of these are the Catalan (Catalonia), which you can see appearing under the Canadian flag (1st col, 36th row). I can't spot the last one.

TheJltres commented 4 years ago

You can get Catalan flag by adding

i.flag.es.ca:before, i.flag.catalonia:before { background-position: 0 -936px }

rubenprodev commented 4 years ago

It's what I already did but you get this output anyway:

Selection_003

I think they should improve it :/

TheJltres commented 4 years ago

Here you have an example: https://jsfiddle.net/TheJltres/9bxmc7rq/1/

rubenprodev commented 4 years ago

Yeah of course, the Flag component, internally uses the "i" html tag, which uses those classes, but if you change one of those flags with the names you say "es ca" or "catalonia", doesn't work. You can try it in there:

https://codesandbox.io/s/4t34j?module=/example.js&file=/example.js:160-166

TheJltres commented 4 years ago

https://codesandbox.io/s/semantic-ui-example-g5wwo

I imported App.css on index.js, and added a flag with the other one.

TheJltres commented 4 years ago

I did a PR in Fomantic-UI, the active fork of Semnatic-UI

https://github.com/fomantic/Fomantic-UI/pull/1549

rubenprodev commented 4 years ago

Yeah thanks, I hope they fix it soon because as you can see in the codesandbox link you provided before, console output show us the warning.

TheJltres commented 4 years ago

Yes, the problem is with types of React-Semantic, now with Fomatic-UI v3 (https://github.com/fomantic/Fomantic-UI/blob/master/ROADMAP.md) will have React types also!

lubber-de commented 4 years ago

Yeah thanks, I hope they fix it soon because as you can see in the codesandbox link you provided before, console output show us the warning.

We most probably won't merge the above mentioned PR. (Technical reasons described at https://github.com/fomantic/Fomantic-UI/pull/1549#issuecomment-652402724) But even, if we would , as you are referring to an error in SUI-React, it would have to be patched there, otherwise you will still encounter the error message you are referring to. If you want this fixed, you have to raise an issue at https://github.com/Semantic-Org/Semantic-UI-React

rubenprodev commented 4 years ago

Thank you, I've posted it in there.