-
Notifications
You must be signed in to change notification settings - Fork 13
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
Disable dragging feature of slide toc #372
Conversation
Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/storylines-editor/issue-323-fix |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary blocker. I know the issue mentions that we should disable this functionality, but I think we should at least investigate a resolution to the issue. Additionally, if we do intend to remove it we should probably get rid of the draggable component.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
1d2373b
to
5450eb4
Compare
I managed to get the dragging of the toc components to work by using the However, I noticed that we're using What would be a good solution for this? Perhaps we could state in the schema that the title of a slide has to be unique, as well as ensuring that the title of new slides are unique before adding them in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was about to suggest we combine the title + slide index as the key for a simple workaround, but I see that's already being done in the existing code: :key="element.title + index"
. Does this not work?
Reviewable status: 0 of 1 files reviewed, all discussions resolved (waiting on @dnlnashed and @szczz)
It seems like the issues go away when I set |
5450eb4
to
eed7b8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean setting the item-key to empty or a random string? I think Yi-Lei's suggestion of title + index would be preferred as we don't want to leave item-key empty. I've tested the dragging functionality and it works well though.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
eed7b8d
to
6fab590
Compare
Yep meant to say |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @IshavSohal)
Related Item(s)
#323
Changes
v-model
attribute withlist
attributeitem-key
to a function that returns a slide's title + index within theslides
arrayTesting
Steps:
This change is