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

Split buttons in the deck editor #3709

Merged
merged 1 commit into from
May 31, 2019
Merged

Conversation

ctrlaltca
Copy link
Contributor

@ctrlaltca ctrlaltca commented May 5, 2019

Related Ticket(s)

Short roundup of the initial problem

The deck editor buttons originally lived in the middle section, where tree filters are now placed.
Now that they have been moved to the left, their usage is counter-intuitive because:

  • add card to deck / add card to sideboard target the card currently selected in the card database (left) pane
  • increment / decrement / remove target the card currently selected in the deck (right) pane

What will change with this Pull Request?

The buttons targeting the deck have been moved in the deck pane.
Other minor style changes have been made to accomodate the change.

Screenshots

Schermata 2019-05-05 alle 13 51 51
Schermata 2019-05-05 alle 13 51 55

@ebbit1q
Copy link
Member

ebbit1q commented May 5, 2019

I'm not sure if I like this change, what do others think?

@tooomm
Copy link
Member

tooomm commented May 5, 2019

I like it a lot when playing with it in the client. 👍
Better than the old placement, logic wise.

Was there a particular reason why you changed the style @ctrlaltca ?
I think the old buttons (a bit bigger & icon only style) are better! That's the way we use them in all other places, too.
Is that due to the removing of deckEditToolBar QToolBar?

Here are the two in comparison:

old
buttons_old
new
buttons_new


Edit: If it helps in some way with the icons if there is no text in the same line, the hash could go at the very bottom below the deck list or directly above the separator and below the comment box.

@ctrlaltca
Copy link
Contributor Author

ctrlaltca commented May 31, 2019

Was there a particular reason why you changed the style @ctrlaltca ?
Is that due to the removing of deckEditToolBar QToolBar?

Exactly.
The extra space taken by the toolbar looked quite weird having only 2 buttons on it.
Anyway, i can revert to the old style if it looks better for you.

@tooomm
Copy link
Member

tooomm commented May 31, 2019

The extra space taken by the toolbar looked quite weird having only 2 buttons on it.

Really? I think it looks much nicer: (I also like the little space between -/+ and X)
qtoolbar

qtoolbar_no

Copy link
Member

@ZeldaZach ZeldaZach left a comment

Choose a reason for hiding this comment

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

I'm very happy with this change and the new layout. Nice job!

@ZeldaZach ZeldaZach merged commit 218aec0 into Cockatrice:master May 31, 2019
@ctrlaltca ctrlaltca deleted the moar-buttons branch June 7, 2019 13:41
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.

The deck minus/plus icons could perform a bit different more intelligently
4 participants