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

Minor chest dialog changes #9028

Open
wants to merge 28 commits into
base: master
Choose a base branch
from

Conversation

Cheerfulbull
Copy link
Contributor

@Cheerfulbull Cheerfulbull commented Aug 4, 2024

Close #9025 and #9024 by centring the XP icon and text and changing the XP message to "XP Needed". One problem: the XP message, when centred, becomes very cramped (comes very close to the edge). What would be the best way to fix this?
Thank you.

@oleg-derevenetz
Copy link
Collaborator

Hi @Cheerfulbull I suggest we limit the scope of this PR to #9025 and create a separate PR for #9024. The idea with the phrase "XP needed:" is questionable in general, because:

  • The string is getting longer without explicit necessity;
  • It is in English that there is a convenient two-letter abbreviation "XP", meaning "experience". In other languages, there may not be such a convenient short abbreviation, that is, we get a localization issue out of the blue.

@Cheerfulbull
Copy link
Contributor Author

Hi @Cheerfulbull I suggest we limit the scope of this PR to #9025 and create a separate PR for #9024. The idea with the phrase "XP needed:" is questionable in general, because:

  • The string is getting longer without explicit necessity;
  • It is in English that there is a convenient two-letter abbreviation "XP", meaning "experience". In other languages, there may not be such a convenient short abbreviation, that is, we get a localization issue out of the blue.

Alright, thank you, I updated the code and removed the text change, the updated PR description would then be:

Close #9025 by centring the XP icon and text.

@oleg-derevenetz oleg-derevenetz added bug Something doesn't work ui UI/GUI related stuff labels Aug 5, 2024
@oleg-derevenetz oleg-derevenetz added this to the 1.1.2 milestone Aug 5, 2024
@oleg-derevenetz
Copy link
Collaborator

If I understand correctly what the issue is, I suppose @LeHerosInconnu will say they're still not centered. With this PR:

chest

@LeHerosInconnu what do you think?

@LeHerosInconnu
Copy link

Hello @Cheerfulbull and @oleg-derevenetz,

If I understand correctly what the issue is, I suppose @LeHerosInconnu will say they're still not centered. With this PR:

chest

@LeHerosInconnu what do you think?

Effectively, you can see even with the naked eye that graphics and texts are not centered horizontally in relation to the respective buttons. :)

Centering chest elements 01

@ihhub ihhub modified the milestones: 1.1.2, 1.1.3 Sep 15, 2024
@ihhub ihhub modified the milestones: 1.1.3, 1.1.4 Oct 23, 2024
@ihhub ihhub modified the milestones: 1.1.4, 1.1.5 Nov 27, 2024
@ihhub ihhub modified the milestones: 1.1.5, 1.1.6 Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work ui UI/GUI related stuff
Projects
None yet
4 participants