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

Solved Default Queen Promotion #1705

Merged
merged 7 commits into from
Jan 25, 2025
Merged

Solved Default Queen Promotion #1705

merged 7 commits into from
Jan 25, 2025

Conversation

SnehalSrivastava27
Copy link
Contributor

It fixed the issue #1704

Here's the video on how it works:

Screen.Recording.2025-01-13.at.3.49.58.PM.mov

Fixes:
Added a Promotion Dialog in Vue Template and CSS to it
Added handlePromotion function to handle the promoting activity

Note: I know we have to not disturb the indentation of original file but honestly the html template was very difficult to understand there were free line gap so I decided to change a little for better future development side and better understanding for a new comer

@llaske llaske changed the title Solved Default Queen Promotion #1704 Solved Default Queen Promotion Jan 14, 2025
@llaske
Copy link
Owner

llaske commented Jan 14, 2025

The UI of the dialog could be improved.
At least a title bar like in tutorial.

image

The icons size could be increase.
The text should be localized.
Please respect indentation of the original file.

@SnehalSrivastava27
Copy link
Contributor Author

Ok
Will work on it

@SnehalSrivastava27
Copy link
Contributor Author

Screenshot 2025-01-16 at 10 23 02 AM Does it look fine @llaske

Original Indentation of file is restored along with that improved ui of promotion box

Added translation of promotion Title
@llaske
Copy link
Owner

llaske commented Jan 16, 2025

It will be better to use the same icons as those in the game and use the color of the player.

image

@SnehalSrivastava27
Copy link
Contributor Author

Screenshot 2025-01-17 at 10 28 16 AM Screenshot 2025-01-17 at 10 28 28 AM

Done

Copy link
Owner

@llaske llaske left a comment

Choose a reason for hiding this comment

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

Please see my comments below.

@llaske
Copy link
Owner

llaske commented Jan 22, 2025

There is still indentation issue here:

image

and here:

image

Don't understand why you can't see it.

@llaske llaske merged commit f3b37d6 into llaske:dev Jan 25, 2025
@llaske
Copy link
Owner

llaske commented Jan 25, 2025

Nice. thanks

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