-
Notifications
You must be signed in to change notification settings - Fork 1
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
[UC17#148#149] Added functionality for users to add their own custom decks #164
Conversation
…arent to child widgets for removing physical cards from decks. Improved User feedback and tests added
…nd add a custom deck. Once added, the deck is added to the page to provide user feedback.
@@ -18,9 +21,12 @@ | |||
|
|||
public class DecksViewImpl extends Composite implements DecksView, ImperativeHandleDeck { | |||
private static final DecksViewImpl.DecksViewImplUIBinder uiBinder = GWT.create(DecksViewImpl.DecksViewImplUIBinder.class); | |||
private static final String DEFAULT_CUSTOM_DECK_TEXT = "Write here name for your custom deck"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd only make a minimal change in the default placeholder, It seems to be a little longer and i'd prefer somehing shorter and clearer... Maybe "Name the deck to create", but it's obviously a minor fixing with no importance relative to the code!!!
@Override | ||
public void onSuccess(Boolean result) { | ||
if (result) { | ||
view.displayAddedCustomDeck(deckName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice attention to details, no need of reloading is a better UX
view.displayAlert("Invalid deck name"); | ||
return; | ||
} | ||
if (pCard == null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to have the error distintion!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've made a good job, i've really appreciated the attention to details even in simple and quick adjutments for a better UX such as differenciating the errors given by the deletion of a card and the fact tha no page reloading is needed to update the decks list after adding a new one.
public void testRemovePhysicalCardFromDeckForInvalidDeckName(String input) { | ||
mockDecksView.displayAlert(anyString()); | ||
ctrl.replay(); | ||
decksActivity.removePhysicalCardFromDeck(input, new PhysicalCard(Game.MAGIC, 111, Status.Good, "This is a valid description"), (Boolean bool) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a minor note, but to improve variety and thus test coverage, I suggest you use if it's possible the random status and game generation method, Status.randomStatus() and Game.randomGame()
This PR implements #148, #149 and adds a new method createCustomDeck in the DecksActivity to make an RPC call and add a custom deck. Once the custom deck is successfully created, it is added to the list of decks on the page.
Tests for the new method were also added.