Skip to content

Conversation

@McGiverGim
Copy link
Member

Fixes #2065

The elements of the Configurator not always adjust to the size of translations. This is specially a problem in very short strings, like the sensors header. This tries to fix that making it resize using a SVG. This is the result:

image

image

@DusKing1
Copy link
Contributor

Wow nice work! But don't you think that GPS is wayy fat?

@McGiverGim
Copy link
Member Author

Yes, it seems it has used all the space, the ideal is to reduce only, not to make it bigger, but I'm not too sure if there is a property for this... I will play a little more with it, but I want to know if this is an accepted solution.

@DusKing1
Copy link
Contributor

It seems nice to me considering there won't be strings longer than 4 CN chars. I think that's acceptable.

@asizon
Copy link
Member

asizon commented Jun 16, 2020

Looks so good this autoadjust idea! Html part cant be done in css??

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch from 8f2dd07 to 364151d Compare June 16, 2020 14:41
@McGiverGim
Copy link
Member Author

Fixed the ugly width and cleaning of the code...

image

image

@McGiverGim
Copy link
Member Author

It seems this latest solution has cutted some of the characters.. I will go back to see what I have lost...

@McGiverGim
Copy link
Member Author

I need to let it by today. The problem is the textLength="100%", if I use it is like the first images, if not it cuts the text. Maybe someone want to play a little more with it today...

@asizon
Copy link
Member

asizon commented Jun 17, 2020

@McGiverGim what about reduce GPS textLength to 70%??Anyway I think the text at 100% in other languages are too big

@McGiverGim
Copy link
Member Author

No that will not work. What this does is to say the text the size it must fill. If I use 70% ALL the text will be compressed to 70% of the assigned space, making too small the big texts.

I will try to find a solution today, but yesterday search give me nothing...

@McGiverGim McGiverGim marked this pull request as draft June 17, 2020 06:56
@McGiverGim
Copy link
Member Author

The SVG path has ended to me... So we have several solutions, now we need to decide which we prefer:

  1. The SVG way, it modifies (reduces and magnifies) all the letters to fit always the size of the div. Is smooth but maybe it makes too bigger the shorter strings.
    image
    image
  2. Cut the text when it is bigger. It has a tooltip, so moving the cursor over the icon will show the complete text. But I don't know if this is acceptable for some languages like Chinese.
    image
    image
  3. Reduce the font when the text is too big. We can do this in an automatic way (the captures are using a javascript library to do it: https://www.npmjs.com/package/textfilljs) or letting the translator add style tags manually to the translations (using the live translation version of the Configurator maybe).
    image
    image

I think this are the more common solutions for this problem.

@asizon
Copy link
Member

asizon commented Jun 17, 2020

For me the 3 solution is the best for these cases :)

@DusKing1
Copy link
Contributor

The third solution seems fine to me. I won't be bothered if I need to add additional style tags.

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch from 364151d to e25837f Compare June 25, 2020 11:09
@McGiverGim
Copy link
Member Author

Updated the code with solution 3 as requested.

Now I think we need to wait @mikeller to see if better to use this code, or better to do the same manually with <style> tags in the translators side.

@McGiverGim McGiverGim marked this pull request as ready for review June 25, 2020 11:11
@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch 2 times, most recently from 953d80b to d69d747 Compare June 25, 2020 11:19
Copy link
Member

@mikeller mikeller left a comment

Choose a reason for hiding this comment

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

Seems to be a reasonable approach. But is this meant to work for all languages? It seems to be failing for some that define overly long text:

image

image

(These are broken in master as well.)

@mikeller mikeller added this to the 10.8.0 milestone Jun 28, 2020
@McGiverGim
Copy link
Member Author

I've limited the minimum don't size but yes, it must work. I will recheck my code to see what is happening.

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch 2 times, most recently from f3dde7e to 370c15b Compare June 29, 2020 11:42
@McGiverGim
Copy link
Member Author

Sorry @mikeller I forgot to upload the library, so it was not working... 🤦‍♂️

I've tested with German and is very large so it does not work at all...
image

The russian works better:
image

With the SVG solution neither...
image

So seeing that, I think the best we can do is simply "cut" the large texts, and if some translator want to make them smaller let him do that in the translations string as you suggested.

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch from 370c15b to fcac8bd Compare June 29, 2020 11:57
@mikeller
Copy link
Member

Yes, the super small fonts do not work - translators will have to change this. Maybe we should add instructions for such fields to indicate that these need to fit into small fields.
But shouldn't they all of these labels be in the same font size in the first place?

@McGiverGim
Copy link
Member Author

No, the library works independent for each field. We can change this, of course but it will not work neither seeing how long are some strings.
We have a text of "keep it very short", and we have the live translator version of Configurator. But it is not enough 😉
If it is ok, I will change it to simply cut the long texts to not break the UI and if someone really needs some space more admit the use of style tags here.

@mikeller
Copy link
Member

No, the library works independent for each field. We can change this, of course but it will not work neither seeing how long are some strings.

That's where I am not sure if trying to do this automatically here to make it work for a small number of languages, and introducing the potential for all other languages to end up with an odd look if different size text is used for fields in the same group is the right thing to do here.
I think as long as these are edge cases and not the norm then using style tags for individual messages in individual languages is a better approach.
And German and Russian translators will have to try harder. ;-) I am sure 'Baro' is understandable in German.

@McGiverGim
Copy link
Member Author

Pushed new code. It simply cuts the text if too big. All the icons have a tooltip with a long string, so is not a problem.

I've uploaded the directly cut version:
image

Is cleaner with ellipsis, but maybe the strings are too short then:
image

@DusKing1 you will need to add your own "reductor" in the the translations, something like: <span style=\"font-size:6pt\">Gyro long text<style>. I suggest to give all of them the same size and that you use the "live" translation version of the configurator, in this way you will see how it looks.

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch from fcac8bd to 50265dd Compare June 29, 2020 14:41
@DusKing1
Copy link
Contributor

DusKing1 commented Jul 1, 2020

@McGiverGim Thank you so much for putting so many efforts on this, I like this solution and have no problem with that!

@McGiverGim McGiverGim force-pushed the toolbar_icons_text_svg branch from 50265dd to 73f8f92 Compare July 7, 2020 13:28
@McGiverGim
Copy link
Member Author

Another PR forgotten, rebased!

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 7, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.2% 0.2% Duplication

@mikeller mikeller merged commit f02564e into betaflight:master Jul 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Translation strings break UI

4 participants