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

Footer style improvements for quiz on learning mode #7106

Merged

Conversation

Imran92
Copy link
Contributor

@Imran92 Imran92 commented Aug 16, 2023

Resolves #7073

Proposed Changes

  • Fixed quiz actions block not adding block attributes during render
  • Fixed Button spacings
  • Updated styles for desktop and mobile as per the new design

Testing Instructions

  1. Check out this branch of Course theme Quiz footer button styles fix themes#7317
  2. Activate course theme
  3. Create a course
  4. Create a lesson in it with a quiz
  5. Enable learning mode
  6. Take the course as a student, go to the quiz page
  7. Make sure the footer looks as expected
  8. Check in both mobile and desktop mode
  9. Now disable learning mode
  10. Make sure it didn't break anything
  11. Now repeat step 5-8 for all variations of course theme, top sensei themes, another block theme

Desktop:
image

Mobile:
image

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@Imran92 Imran92 added this to the 4.16.2 milestone Aug 16, 2023
@Imran92 Imran92 requested a review from a team August 16, 2023 08:53
@Imran92 Imran92 self-assigned this Aug 16, 2023
@github-actions
Copy link

github-actions bot commented Aug 16, 2023

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit ca30787 and trunk. Please review and confirm the following are correct before merging.

No changes detected in the current commit. But the comment was left so it is possible to check for the edit history.

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

❗ No coverage uploaded for pull request base (feature/frontend-improvements@d12743d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                       Coverage Diff                        @@
##             feature/frontend-improvements    #7106   +/-   ##
================================================================
  Coverage                                 ?   49.35%           
  Complexity                               ?    10546           
================================================================
  Files                                    ?      575           
  Lines                                    ?    44546           
  Branches                                 ?      402           
================================================================
  Hits                                     ?    21987           
  Misses                                   ?    22232           
  Partials                                 ?      327           

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d12743d...ca30787. Read the comment docs.

@donnapep
Copy link
Collaborator

@Imran92 As per our 1:1 chat the other day, I think this one is still pending changes to use padding and not line height for buttons so that the quiz buttons are styled similarly to the lesson buttons. Do I have that right?

@Imran92
Copy link
Contributor Author

Imran92 commented Aug 17, 2023

@Imran92 As per our 1:1 chat the other day, I think this one is still pending changes to use padding and not line height for buttons so that the quiz buttons are styled similarly to the lesson buttons. Do I have that right?

Yap! You're right! I'll be on it very soon

@donnapep donnapep linked an issue Aug 18, 2023 that may be closed by this pull request
Base automatically changed from add/change-quiz-design-learning-mode to trunk August 18, 2023 17:57
@donnapep
Copy link
Collaborator

donnapep commented Aug 18, 2023

I'm not sure if we want to keep the stickiness on the footer. I've asked about that in Slack - p1692382503101569-slack-C013QUH20TS.

Something else I noticed is that the Complete Quiz button is not left-aligned with the quiz title:

Screenshot 2023-08-18 at 2 23 04 PM

@donnapep
Copy link
Collaborator

donnapep commented Aug 18, 2023

After I submit the quiz and am either awaiting a grade or the quiz was auto-graded, the buttons in the footer are broken on desktop and mobile:

Awaiting Grade - Desktop

Screenshot 2023-08-18 at 2 41 42 PM

Awaiting Grade - Mobile

Screenshot 2023-08-18 at 2 42 13 PM

Graded - Desktop

Screenshot 2023-08-18 at 2 42 44 PM

Graded - Mobile

Screenshot 2023-08-18 at 2 43 03 PM

@donnapep
Copy link
Collaborator

Note that there are some upcoming PRs for the footer when awaiting a grade #7101 and for graded quizzes #7116 where you can see what the footer should look like, so if you'd rather address these issues as part of one of those PRs, that's fine with me. We'd just need to target a feature branch for this PR so we don't merge anything broken to trunk. 🙂

@Imran92
Copy link
Contributor Author

Imran92 commented Aug 27, 2023

so if you'd rather address #7106 (comment) as part of one of those PRs, that's fine with me. We'd just need to target a feature branch for this PR so we don't merge anything broken to trunk.

Thanks Donna! I've looked into the design issues in footer in 'awaiting grading' state and looks like it's better to handle them in a separate PR under a different issue. Also looks like the broken issue was not a side effect of this PR, it's been looking like that in trunk for quizzes with awaiting grade/graded state that have reset enabled. Here's a screenshot from trunk. So we'll probably not be breaking anything new in trunk

Screenshot 2023-08-28 at 7 06 08 AM

Having looked into it, it's because quizzes are rendered using the quiz template only at the time of being taken by the student, whereas both of the 'awaiting grade' and 'graded' states of quizzes use the lesson template instead. The lesson template comes with the 'lesson actions' block in the footer which renders the 'Complete Lesson' button at the bottom. The 'restart quiz' comes from a 'page actions' block. The new design requires to hide the outputs of 'lesson actions' too, and then add a 'Pending Teacher Grade' button (Or maybe we can change the content of Complete Lesson button conditionally and make it disabled, but does not look like a good idea to me).

So, overall, as this awaiting grade is connected to a separate template with a different footer, I think it's better to address the things I've stated above in a separate PR

@Imran92 Imran92 changed the base branch from trunk to feature/frontend-improvements August 28, 2023 00:49
@donnapep
Copy link
Collaborator

donnapep commented Sep 5, 2023

@Imran92 I tested with Learning Mode disabled too and it looks slightly better. Nothing appears to be broken at least. 😄

I also 16004b8 to match the design since the footer was too tight to the bottom of the page. Once this PR and question styles are merged, I think some more whitespace tweaks will be needed to ensure the space between the bottom of the last question and the top of the quiz actions is correct.

Something else I noticed is that the Complete Quiz button is not left-aligned with the quiz title:

This hasn't been fixed yet. I need to move on to some other things, but I may try to revisit it soon so we can get this one merged. I believe that's the only outstanding issue for this PR at this point. 🤞🏻 Also more than happy if you beat me to it. 😃

@Imran92
Copy link
Contributor Author

Imran92 commented Sep 6, 2023

Something else I noticed is that the Complete Quiz button is not left-aligned with the quiz title:

This hasn't been fixed yet. I need to move on to some other things, but I may try to revisit it soon so we can get this one merged. I believe that's the only outstanding issue for this PR at this point. 🤞🏻 Also more than happy if you beat me to it. 😃

Oh, sorry I forgot to mention that here, I fixed the alignment issue in the question style update PR as the alignment issue was affecting the questions too #7124 . So this is also resolved ☺️

Thanks @donnapep !

donnapep
donnapep previously approved these changes Sep 6, 2023
Copy link
Collaborator

@donnapep donnapep left a comment

Choose a reason for hiding this comment

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

🥳

@Imran92 Imran92 merged commit c4052cc into feature/frontend-improvements Sep 7, 2023
23 checks passed
@Imran92 Imran92 deleted the add/footer-style-update-lm-quiz branch September 7, 2023 07:37
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.

Update quiz footer in Learning Mode
2 participants