0xFableOrg / 0xFable

A fully on-chain trading card game. There will be elves, wizards & shit. Drama and broken friendships also.
https://twitter.com/0xFableGame
BSD 3-Clause Clear License
103 stars 35 forks source link

The (Editor and Collection) Merge #108

Closed eviterin closed 4 months ago

eviterin commented 6 months ago

Context (Problem, Motivation, Solution)

Editor was added in my previous PR. This editor was based on the Collection view, and shared a lot of component and functionality. Instead of maintaining the two, I now combined them.

Merge Collection and Editor

Describe Your Changes

Both Collection and Editor had these two components: On the left: Search/Filter panel Middle: Card collection

In Collection: On the right: Player's completed decks

In Editor: On the right: Player's deck that is under construction

Each of these components have been factored out into their own component. Editor has been removed, and the Collection now keeps track on whether the player is in editing mode or not. Index file has been updated to remove the editor shortcut.

Additionally, variables within the collection view have been reordered and grouped by the functionality they support.

Checklist

Testing

Exploration testing to make sure there are is no regression within the Collection view.

norswap commented 6 months ago

Is this ready for review? Just tried it and seems to have quite a few problems (e.g. deck doesn't appear in list when you add a new deck) + unused functions in collections.tsx.

Tip: you can mark the PR as "draft" if it's not ready for review (that way other people can have a look if they want to, but they know it's not blocking!)

eviterin commented 6 months ago

I ran eslint and resolved all warnings. Sorry for that will try to do better in the future..

@norswap I tried cloning my PR fresh and still cannot reproduce the issue you're having with decks not appearing in the list. The following steps I take:

  1. Navigate to http://localhost:3000/?index=1
  2. Select Collection
  3. Select New Deck
  4. Add cards and a name and press Save
  5. Assert that the deck I created is on the deck list.

Keeping this issue in draft until I know more.

norswap commented 6 months ago

Might be on my side, maybe I need to rerun make setup, will report back!

norswap commented 6 months ago

I can reprod, both on Chrome & Firefox, even if I clone fresh.

To be clear, after creating the deck, I expect it to show in the right sidebar, instead, only "New Deck" is shown.

eviterin commented 6 months ago

Thanks for the review @norswap. I have now added commits for each actionable comment (reacted with a 👍)

Edit: not ready for re-review. Make check is killing me

eviterin commented 5 months ago

Minor CSS issues now with Aritra's changes, but will be fixed in upcoming PR for onchaining decks.

norswap commented 4 months ago

Global feedback:

Still, this is pretty usable, so I'm going to merge this now, so we can enable prettier globally. Then I think the next priorities should be: