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

Increasing scrolling room for question lists in MultiPaneLayout #8143

Merged
merged 4 commits into from Jun 11, 2021

Conversation

sairina
Copy link
Contributor

@sairina sairina commented Jun 7, 2021

Summary

Removed the maxHeight calculations for the MultiPaneLayout container because the original issue (#6945) is partially caused by the header of this component being non-responsive, and thus, when the window height is reduced, the header continues to take up the same amount of space in the visible window (also seen in #6900), which causes the remaining portions (the aside and main parts) of the component to shrink dramatically to fit. Removing the calculation allows for the bottom portion to be fully visible on page scroll.

Because this component is used in several places, changes are also documented below. There is also tech debt in this component, and that has been filed as a separate issue.

LessonContentPreviewPage (The main issue)

NOTE: The reason this particular component has major problems with the MultiPaneLayout is because of all of the information in the header. Other components tend not to have such an obvious issue.

Before After
2021-06-07 16 20 23 2021-06-07 16 19 01

ExamReport

Before After
2021-06-07 16 15 45 2021-06-07 16 16 51

LearnerExercisesReport

Before After
2021-06-07 16 12 39 2021-06-07 16 11 59

QuestionLearnersReport

Before After
2021-06-07 16 05 42 2021-06-07 16 03 30

References

Reviewer guidance

To see the original issue, LessonContentPreviewPage

  1. Log in as Coach
  2. Click on "Plan" >"Quizzes" > "New quiz"
  3. Make a new quiz, and while previewing it to choose questions, change the size of the window. You should still be able to see all the questions in the question list, by scrolling through the page scroll.

ExamReport

  1. Sign in as learner
  2. Click on Quizzes
  3. Click on a quiz that you have completed to see a report

LearnerExerciseReport

  1. Log in as coach
  2. Click on Reports > Lessons (click on a lesson) > Report (click on a lesson) > Report (click on a learner's name)
  3. View the LearnerExerciseReport

QuestionLearnersReport

  1. Login as coach
  2. Click on Reports > Lessons (click on a lesson) > Difficult questions (click on a question)
  3. View the individual question, which will be the component, QuestionLearnersReport

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@sairina sairina marked this pull request as ready for review June 8, 2021 02:41
@sairina sairina added the TODO: needs review Waiting for review label Jun 8, 2021
@indirectlylit
Copy link
Contributor

This looks like a big improvement to responsiveness!

Quick question: it looks like there might be an issue with missing icons or unexpected empty space in the ExamReport and LearnerExerciseReport gifs – is this a known issue?

image

@sairina
Copy link
Contributor Author

sairina commented Jun 8, 2021

@indirectlylit re:

Quick question: it looks like there might be an issue with missing icons or unexpected empty space in the ExamReport and LearnerExerciseReport gifs – is this a known issue?

I did notice those empty spaces. I haven't been able to find those as known issues, but I don't actually know what it should look like. I'm happy to file an issue, but I would need some help to complete the "Expected Behavior" portion of the issue.

@sairina sairina linked an issue Jun 8, 2021 that may be closed by this pull request
@indirectlylit
Copy link
Contributor

indirectlylit commented Jun 9, 2021

I think we used to show the notStarted icon there because those rows are for 'no attempts made' questions

Copy link
Contributor

@jonboiser jonboiser left a comment

Choose a reason for hiding this comment

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

Looks good! I made an issue for the missing icon bug.

@jonboiser jonboiser merged commit 56482a4 into learningequality:develop Jun 11, 2021
@jonboiser jonboiser removed the TODO: needs review Waiting for review label Jun 11, 2021
@jonboiser jonboiser added this to the 0.15.0 milestone Jun 11, 2021
@indirectlylit
Copy link
Contributor

indirectlylit commented Jun 11, 2021

thanks both!

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

Successfully merging this pull request may close these issues.

Exercise question preview desperately needs more scrolling room
3 participants