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

feat: card atom #16

Merged
merged 2 commits into from
Aug 12, 2020
Merged

feat: card atom #16

merged 2 commits into from
Aug 12, 2020

Conversation

guilhermevrs
Copy link
Contributor

@guilhermevrs guilhermevrs commented Aug 8, 2020

Objective

The idea here is to have a base card component and to extend it for the real cards.
This would enable for example to extend the card-ui to have:

  • Normal playing cards
  • Tarot playing cards
  • etc

This PR includes then:

Missing actions

  • Need the linting / formating
  • Need the CI
  • Squash once finish

Closes #16

src/models/BaseProps.tsx Outdated Show resolved Hide resolved
@guilhermevrs guilhermevrs changed the title feat: BaseCard molecule feat: base card atom Aug 9, 2020
src/index.tsx Outdated Show resolved Hide resolved
@guilhermevrs guilhermevrs changed the title feat: base card atom feat: card atom Aug 9, 2020
@guilhermevrs
Copy link
Contributor Author

image

@tib-tib
Copy link
Contributor

tib-tib commented Aug 10, 2020

In my mind I was picturing the Card component to be much simpler, something like:

return (
  <img src={isFront ? frontFace : backFace } />
)

, with frontFace and backFace being two svg urls. And that was it haha!

I don't think having a generateSvg method makes sense, because depending on the kind of cards we would have (Tarot, UNO, whatever) we might not have the same way of retrieving/generating them.
At some point we might even get a deck of cards from the backend and it would be a list of something like Card: { value: 'ace', color: 'spades', url: 'url-of-my-svg-card' }.

Anyway, I think what you did is perfectly fine, just wanted to express my thoughts :)

@guilhermevrs
Copy link
Contributor Author

Well, what I was thinking for the PlayingCard is to be an extension of BaseCard.
So BaseCard could be any card, since the only thing you can do is to place it somewhere and turn it via click.

I called PlayingCard because I was referring to a "normal playing card". I would imagine a UnoCard or TarotCard if it's the thing. Specially because the type of entry would be different (I mean, you wouldn't instantiate the UnoCard with JokerBlack).

Now about using the img tag. You can only do it if you have an SVG separated for each card. In this one, it's a SVG Sprite (so a single download can get you all the cards already). Since it's an SVG sprite, you can't simply use the <img src= I guess.
Moreover, using img src blocks you of using SVG inline attributes (like fill for example).

At server level, indeed, you could receive the data (like the SVG url). But then would that be a single SVG or an SVG sprite?
If it's a single, we could have a mode for loading the card from single SVG (in that case, two svgs, one for the front and one for the back).

Again, let's discuss during our call

@tib-tib tib-tib merged commit fc383fb into master Aug 12, 2020
@tib-tib tib-tib deleted the feat/3-create-card branch August 12, 2020 18:26
@guilhermevrs
Copy link
Contributor Author

Closes #3

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.

2 participants