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

Restrict the width of the player info table on the player detail page #1696

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

Conversation

smismib
Copy link

@smismib smismib commented Feb 6, 2023

Texts are too long if wage options are enabled, causing the problem in the div on its right in certain languages (Chinese and Japanese at least as far as I know).

Restrict the width of player info table on player detail page if the skills are visible under the new design.

Current Chinese version behaviour
current

After the fix
fix

Current English verison
english_current

English version after the fix
english_fix

@minj
Copy link
Owner

minj commented Feb 12, 2023

Thank you for your PR but I do not think this is ready to be merged as is.

Are you sure this is not caused by either of the following?:

  • RTL CSS problem
  • Parsing failure (I see "undefined" in the screenshot)

I do not agree with your solution of adjusting CSS in a random util function.

My suspicion is this needs to live next to the wage feature.

Please take a look at Foxtrick.Pages.Player.getWage referenced by ExtendedPlayerDetails module. It's likely where the problem actually lies.

@smismib
Copy link
Author

smismib commented Feb 12, 2023

  • RTL CSS problem
    My language is not RTL. I haven't looked at the code RTL support. I am not sure how RTL could potentially affect this.

  • Parsing failure (I see "undefined" in the screenshot)
    The parsing failure always shows up for me. I can take a look, find out what goes wrong, and make a fix if possible.

The problem is regarding the width of the section. Wage is one part of it. The other part which may make it too wide is a row called "U21 World Cup Eligibility". See the screenshot (and it's without the parsing error)
Screen Shot 2023-02-12 at 12 11 14

May I have your suggestion on this since changing things in Wage alone won't let us get rid of the problem?

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