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

add more info to dialogs #4293

Merged
merged 4 commits into from
Apr 2, 2021
Merged

add more info to dialogs #4293

merged 4 commits into from
Apr 2, 2021

Conversation

ebbit1q
Copy link
Member

@ebbit1q ebbit1q commented Mar 24, 2021

adds descriptive strings to the register, password reset request,
password reset challenge request, password reset token dialogs
adds tip to set manager to use ctrl a to select all sets
change sizes in set manager
moves default server info to settings instead of having it hardcoded in
each dialog

default size of sets manager is now this
image

tr("Only cards in enabled sets will appear in the card list of the deck editor") + "<br><br>" +
"<b>" + tr("Card Art") + ":</b> " + tr("Image priority is decided in the following order") +
"<ol><li>" + tr("CUSTOM Folder") +
labNotes->setText(tr("Use ctrl+a to select all sets in the view.") + "<br><b>" + tr("Deck Editor") + ":</b> " +
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
labNotes->setText(tr("Use ctrl+a to select all sets in the view.") + "<br><b>" + tr("Deck Editor") + ":</b> " +
labNotes->setText(tr("Use CTRL+A to select all sets in the view.") + "<br><b>" + tr("Deck Editor") + ":</b> " +

Regarding all hintsGrid:
Don't you think the extra line breaks were useful?
Because of how lists are displayed you now have two blocks, but the second block being bullet points of the last element of the first block. The connection and context is not as clear now as seen in your screen.

Yours:

Compared to what is currently in client:
hints

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to make it smaller, it's taking up a lot of space

Copy link
Member

Choose a reason for hiding this comment

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

I see, sure. Both not ideal. :)

@ebbit1q
Copy link
Member Author

ebbit1q commented Mar 25, 2021

image
I made the dialog hint a little smaller again

@ebbit1q
Copy link
Member Author

ebbit1q commented Mar 25, 2021

in order to make the list smaller I stopped disabled sets from being used for image art, this resulted in a >1% performance increase as shown with the added timer

adds descriptive strings to the register, password reset request,
password reset challenge request, password reset token dialogs
adds tip to set manager to use ctrl a to select all sets
change sizes in set manager
moves default server info to settings instead of having it hardcoded in
each dialog
@ebbit1q
Copy link
Member Author

ebbit1q commented Apr 2, 2021

I removed a few needless entries to the server dialog settings, among which a password entry that saves the password you used last in the register or recover dialog, whether you want it or not............
None of these entries are used anymore since the new login dialog was added, even though they all accounted for entries to be used by the old dialog they now do nothing.
Its possible to adjust these changes to try to mess with the new login dialog but imo it's not worth it because it doesn't in the current state.
I also fixed the remove function, which used to remove an entry but not move all other entries back by one.

@ZeldaZach ZeldaZach merged commit 8e954b1 into Cockatrice:master Apr 2, 2021
@ebbit1q ebbit1q deleted the dialog_info branch April 10, 2021 14:13
@tooomm tooomm mentioned this pull request Apr 11, 2021
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.

None yet

3 participants