Deck Editor added

[eviterin]

Context (Problem, Motivation, Solution)

There needs to be a deck editor in place (see: https://github.com/0xFableOrg/0xFable/issues/40). Index page updated to redirect to editor. Which has heavily borrowed content from the collections page.

Describe Your Changes

• Allow user to select cards from a collection and add it to their deck
• Allowsuser to select cards already in the deck to remove them from the deck.
• Rename deck
• Discard deck
• Big card is now updated on hover instead of on select

What's not included? A place to view the player's decks (I'm thinking collections page) Logic to save the deck (no onchain transaction)

Checklist

• [X] I have performed a self-review of my code
• [X] I ran make check and fixed resulting issues
• [X] I ran the relevant tests and they all pass Ran linter and resolved all warnings and errors.
• [ ] I wrote tests for my new features, or added regression tests for the bug I fixed Exploratory testing only. Should I be writing tests in e2e using playwright?

Testing

1. Browse to http://localhost:3000/?index=1
2. Select Editor
3. Press save
4. Assert unable to save with empty name
5. Add a couple of cards to deck
6. Rename deck
7. Press save
8. Assert no error to save
9. Press discard
10. Assert all cards removed

[norswap]

Hey Eviterin! Thanks for contributing!

Could you split the diagram and the deck editor into two different PRs to make it easier to review / faster to merge?

Quick feedback on the diagram:

• could you distinguish the game creator from the joiner (just setting "Player A initiates createGame" for instance)
• the joiner cannot cancel the game at the moment, only the creator
• would be good to import the picture directly into the repo instead of linking to a third-party website

I'm semi-off this week, but I'll try taking a look at the deck editor later this week.

[eviterin]

Thanks for the feedback.

Against all odds I have been able to split up the pull request in two. This one is now related to Deck Editor. And this is related to activity diagram: https://github.com/0xFableOrg/0xFable/pull/97

[norswap]

Exploratory testing only. Should I be writing tests in e2e using playwright?

We haven't really been doing this in a while, so it's fine. We probably ought to rewrite playwright tests to not use Metamask in most cases (as that makes it very slow and it's more fragile when updating etc). Future work :')

[norswap]

This is pretty nice!

The collection page was also pretty much exploration work, and is even uglier than the rest of the game. We probably won't feature it in the proof of concept. Nevertheless, here's an issue with some ideas of what that facelift could look like.

Adding the deck editor is an interesting additional constraint. I like what you've done, and I think we could integrate it into the proposal above with a right panel (but that just displays when visualizing or editing the deck). One thing I would change is to replace the full-size display of cards in the right panel with a "list" view where only the name of the card (and in time, some logo/colors/decorations for its main characteristics, e.g. creature, color, ...). We could display the full card on hover (either inline or in a popup).

UX Issues

Some UX problems I've seen in testing:

• adding more than a few cards to the deck causes a sidebar to appear (good) which causes the size of the cards to change (bad)
• renaming a deck causes a new deck to be created, but not the old one to disappear
• when editing an existing deck, the deck name is not populated
• the clear button is a little bit confusing, it's not quite clear it will remove all cards in the deck
• no easy way to get back to the collection view
  • I would probably add a button to abort deck creation, which would replace the clear button and navigate back to the collection view
  • though as mentioned in the comments above, I think both should be a single page

Card Details

A question I had was how to handle card details display. Right now we can display all details pretty easily but long term that won't be the case.

A hover display works, thought it'd be nice to be able to keep a card displayed and compare with others? Maybe that isn't really needed.

Also where to put that display? In the collection UX overhaul issue, we haven't budgeted space for displaying the card on the left as exists currently. Maybe we could reintroduce that (split the left panel in two halves — maybe allowing to minimize the top half (lists of decks & filters) for small screens.

We could have a floating hover, but that could be janky when trying to move away from it? Maybe it works.

Another option is to require a click for expanded display then have a "Add to Deck / Remove From Deck" button on the expanded display.

Next Steps

I see two ways to build on this:

• UX overhaul following the issue & consideration above (and further discussion based on your own ideas!), merging collection/editor
• Actually querying the chain for getting and creating the decks. Probably add deck names on chain too! Could also implement the incremental display updates described in the "Data Architecture" section of the issue

[eviterin]

Hey Norswap. Thank you for the awesome feedback and for sharing your thoughts. To start, I fully agree on merging collection/editor. There's so much overlap.

There are some things you mention that I don't really have much experience with and might need some pointers on. Marked with (!).

Would you prefer these changes in one PR or can I split it up in three, as proposed below.

40 related, short term:

Replace cards with their names - keep hover. Prevent cards in deck to change size when sidebar appears. Rename deck to remove old deck. Auto populate the deck name when editing an existing deck. Remove the clear button. Shortcut to collection view from editor view.

40 related short/medium term:

(!) Add deck names on chain Try out different display options(floating hover, show/hide filters, not having it at all?)

93 related, medium term:

Merge collection and editor view. (!) Add filter for inventory cards (!) List all cards owned

Anything I'm missing?

[eviterin]

Fixed the short term stuff.

Although there is a known error: when loading an existing deck, it does not highlight the cards that are in the deck.

Seems that since we are using index to identify cards and we have two views (collection and editor) the ids don't match up. ID 0 in collection is ID 24 in editor, 1 in collection is 25 in editor. And so on - since we have 24 cards.

This issue should resolve itself once editor and collection have been merged.

[norswap]

So I'd say before merging this:

• remove semicolons
• change "index" URL parameter to something else like "deckID"

The rest of the work can come in the subsequent PR!

Oh also, let's try to avoid merge commits in branches.

Actually right now we rebase everything (even PRs are merged by rebase), but I'm wondering about merging PRs with merge instead of rebase instead.

[norswap]

One UX note: the buttons overflow to the right of the textfield, creating a lateral scrollbar — we should avoid that!

[eviterin]

Fixed the semicolon stuff and the index renaming!

[norswap]

Approved, but needs to be rebased to be mergeable!