-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add interactive-image panel #525
Conversation
Your demo site is ready! 🚀 Visit it here: https://ramp4-pcar4.github.io/story-ramp/interactive-image/#/en/00000000-0000-0000-0000-000000000000 |
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.
Looks and works great! Tested on Chrome & Firefox with keyboard and on mobile. The only issue I found is that on mobile the text overlaps with the image as you scroll down:
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @spencerwahl)
src/components/panels/interactive-image-panel.vue
line 127 at r1 (raw file):
onMounted(() => { if (props.slideIdx && props.slideIdx > 2) {
This should probably take into account the lazyLoad
flag, which disables lazy loading if set to false. You should just need to add in props.lazyLoad &&
here, and then add the following to the props variable up top (the prop should already be passed in to all panels so nothing more is needed):
lazyLoad: {
type: Boolean
}
src/components/panels/interactive-image-panel.vue
line 186 at r1 (raw file):
const zoneMouseEnter = (zone: InteractiveImageZone): void => { console.log(zone);
Stray console.log
2364041
to
c56c4ba
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.
Huh I must have accidentally removed the fix for that. Donethanks.
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @RyanCoulsonCA)
src/components/panels/interactive-image-panel.vue
line 127 at r1 (raw file):
Previously, RyanCoulsonCA (Ryan Coulson) wrote…
This should probably take into account the
lazyLoad
flag, which disables lazy loading if set to false. You should just need to add inprops.lazyLoad &&
here, and then add the following to the props variable up top (the prop should already be passed in to all panels so nothing more is needed):lazyLoad: { type: Boolean }
done
src/components/panels/interactive-image-panel.vue
line 186 at r1 (raw file):
Previously, RyanCoulsonCA (Ryan Coulson) wrote…
Stray
console.log
done
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.
I'm still seeing the same behavior shown in the GIF
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @RyanCoulsonCA)
c56c4ba
to
db2cad7
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.
Ok it should actually be fixed this time, sick me put the class on the wrong element 🏆
Reviewable status: 4 of 5 files reviewed, 2 unresolved discussions (waiting on @RyanCoulsonCA)
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 r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @spencerwahl)
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 4 of 5 files at r1, 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @spencerwahl)
Changes
interactive-image
panel, to fix issues with the hacky implementation in a project and to make it reusable.Notes
A large portion of the panel file is taken from dynamic panels. The change looks large but it shouldn't be too crazy to look through.
Testing
Steps:
Please test in mobile, keyboard etc. Things worked for me but who knows what happens when things become a demo.
This change is