Skip to content
This repository has been archived by the owner on Jan 12, 2021. It is now read-only.

Rewrite suggestion #49

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Rewrite suggestion #49

wants to merge 2 commits into from

Conversation

chrillewoodz
Copy link
Collaborator

This is a working example which fixes a lot of the bugs that I've reported, I've also got some more suggestions that would make it better and more flexible for different use cases. As well as features that has to be added. However that is not relevant to this pull request.

@Viczei
Copy link
Owner

Viczei commented May 26, 2017

Ok nice. i'm testing it and will leave a comment if i've questions.

switch (action) {
case 'like': this.onLike.emit({like: true, card: this.card}); break;
case 'dislike': this.onDislike.emit({like: false, card: this.card}); break;
case 'special': this.onSpecial.emit({special: true, card: this.card}); break;
Copy link
Owner

Choose a reason for hiding this comment

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

what is the use of the 'special' action? It seems to have no behaviors in the components apart from sending a callback with the card element.

Copy link
Collaborator Author

@chrillewoodz chrillewoodz May 26, 2017

Choose a reason for hiding this comment

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

It's supposed to be like a "super like" or whatever you want it to be. I've got some ideas how we can make this better, by instead of having like/dislike etc we have left/right/top/bottom swipes instead. And then you can decide on your own what you want each direction to do. This would make it really easy to customise the behaviour.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now it only fires from a button using a directive. That's why you can't see any behaviour attached to it, it basically just changes the overlay color.

Copy link
Owner

@Viczei Viczei May 26, 2017

Choose a reason for hiding this comment

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

I see what you mean, so the special is just an example and we can add for example left, top, ... etc actions
It's indeed more logical than a like or dislike which would not always be the use of a tinder card even if the most common

Copy link
Owner

Choose a reason for hiding this comment

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

If the plan is to make left/right/top/bottom actions, wouldn't it be better to make action and direction the same argument for the function?
And treat "like" as a "right" and "dislike" as a "left", in order to prevent regressions for current behaviors.

I don't really see the advantages of the "special", if it's needed to change the overlay component, i think it's already possible through the config input.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think if we specify exactly what we want to do it'll be easier to understand. The action should indeed be left, right etc. instead of like/dislike. I don't think we need to think a lot about regression since the repo isn't that big yet. Now we have the chance to make it right :)

Copy link
Owner

Choose a reason for hiding this comment

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

yes i think too that we should specify a bit all this. I also had plans for the future by adding a drag and drop system to cards (tinder cards beeing a drag and drop system)
I did begin a work on the branch card-grid

@Erim32
Copy link

Erim32 commented Apr 13, 2019

Hi, any update about all of this ? :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants