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

removed scroll on adding new question #4181

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
<transition-group name="list-complete" tag="div">
<VCard
v-for="(item, idx) in sortedItems"
ref="questionCardRef"
:key="`question-${item.assessment_id}`"
pa-1
class="elevation-4 list-complete-item"
Expand Down Expand Up @@ -333,6 +334,17 @@
this.$analytics.trackAction('exercise_editor', 'Add', {
eventLabel: 'Question',
});
this.$nextTick(() => {
const questionCards = this.$refs['questionCardRef'];
if (questionCards?.length >= 1) {
const lastQuestionCard = questionCards[questionCards.length - 1].$el;
const editorDiv = document.getElementById('editViewId');
editorDiv.scrollTo({
top: lastQuestionCard.offsetTop,
behavior: 'smooth',
});
}
});
},
async deleteItem(itemToDelete) {
if (this.isItemActive(itemToDelete)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,13 +285,6 @@
if (!this.question) {
this.openQuestion();
}
// Assessments are nested inside of a scrolling panel.
// Instead of propagating an event all the way back to
// the scrolling panel, just use scrollIntoView
// (supported by most major browsers)
if (this.$el.scrollIntoView) {
Copy link
Member

Choose a reason for hiding this comment

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

Hi @Jaspreet-singh-1032, thank you for the changes here. Although I have some queries, removing this if statement would completely eliminate the smooth animation effect. Instead, we may want to enable scrolling with animation to the "response type" field rather than the "title/question". @MisRob @LianaHarris360 can you guide on the exact behaviour here? Removing the if statement does seem to work altho there won't be any scroll animation.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think we would want to keep the animation scroll. As for enabling scrolling to the "response type" field, the scrollIntoView() method takes in a block argument in addition to behavior, that defines the vertical alignment and the default is set to 'start' which doesn't seem to scroll to the preferred view, so possibly trying 'center' or 'nearest' could be an option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The maximum of the response type shown by using block: end or nearest.
This is how it looks.
image

Copy link
Member

@MisRob MisRob Jul 24, 2023

Choose a reason for hiding this comment

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

It seems to me that it's caused by the secondary bar (with "Details", "Questions", "Related"). Even though the scroll position is correct, the element we scrolled into view is hidden underneath the bar. I think that one way to deal with it might be something like:

  1. Get coordinates of the question type element
  2. Scroll window (https://developer.mozilla.org/en-US/docs/Web/API/Window/scrollTo) to those coordinates but with an offset that corresponds to the bar height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I will try this solution. However, the window.scrollTo function was not working here, but I will research it. It could be due to some CSS.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yes, this is a modal actually, so I assume that's the reason. Perhaps Element.scrollTo() https://developer.mozilla.org/en-US/docs/Web/API/Element/scrollTo could be used then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scrollTo did not work on any of the elements. I think this is because there is an outer scrollable div in which all these elements are rendered and the scrollTo only works on the outer parent div and not on the internal elements.

Copy link
Contributor Author

@Jaspreet-singh-1032 Jaspreet-singh-1032 Jul 26, 2023

Choose a reason for hiding this comment

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

I was able to scroll using the outer div with scrollTo function. I think I can implement a workaround using it 💡

Copy link
Member

Choose a reason for hiding this comment

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

Ah, that sounds tricky. Wonderful you found a workaround, let's see. Let us know if you needed anything.

this.$el.scrollIntoView({ behaviour: 'smooth' });
}
},
methods: {
updateItem(payload) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<template>

<VContainer ref="editview" fluid class="pa-0 wrapper" @scroll="scroll">
<VContainer id="editViewId" ref="editview" fluid class="pa-0 wrapper" @scroll="scroll">
<VContainer v-if="!nodeIds.length" fluid>
<VLayout justify-center align-center fill-height>
<VFlex grow class="grey--text text-xs-center title">
Expand Down