-
Notifications
You must be signed in to change notification settings - Fork 35
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
Prepending card if added at the beginning #7
Conversation
Thanks for this, I'll look into it later today or over the weekend. |
I didn't tested it, but there should be still issue for some cases. For example if you add two cards at the same time, then only one of those will have index 0, so one will be added at the beginning, and the other at the end, because the further will be at index 1. So what we should check instead is whether index of added card is lower that amount of added cards. For example when there are two cards added, then indexes will be as follows: 0 and 1. Both of those are lower than amount of added cards (2) so those will be prepended. As the are no automated test in this repository I prefer not to mess much with so I tried to keep change as small as possible. |
Ah, I didn't know this was how Swing was implemented. This doesn't solve the issue of when we add a card in the middle of the stack, right? Do you think there's a more elegant way to do this? Otherwise, I'll test this out later and get back to you. Yes, I don't think the |
I think that swing doesn't support such case, you can either append or prepend card. And I think that it's okay, as I can't imagine scenario where someone would like to add card at the beginning. My case is that I have only three cards visible at the time, so once first one is thrown away then I predend another one so I still have three.
I would first add tests for that repository, because It's quite dangerous to change anything, as I never know if I introduced some regression by my change. |
I think I'm okay with this implementation if there's a note in the README about how we don't support adding elements in the middle of the stack. If you want to add some documentation, I'll merge it in as well. Otherwise, I'll add it in before the next release. I wanted it as a separate PR because I didn't want unrelated changes in this PR. After some manual tests, I found that the Otherwise, looks good to me. |
I will leave documentation up to you as we probably might have different approach to it. In general a swing library doesn't support adding card in the middle, so I can't imagine that wrapper might do. I reverted the changes so card field is available again. Cheers. |
Closing as no response from you. I'm gonna release the fork: https://github.com/adelura/vue-swing2 |
I'll merge your changes separately and release it in the next version since the branch has changed. Thanks for contributing! |
Thank you for such a nice wrapper for swing library! It works well. Therefore I found some small issue.
When adding a card to stack swing expose an option to prepend one. Currently, your wrapper allows only appending cards. It's not always the case. There is a scenario when the card is added at the beginning. To handle that, I check if the index of an added card is at 0. It means that card should be prepended.
Also, I removed
this.card
field because it's not used, and I was confused initially why we need this. I can remove that change if you want - I don't mind.