-
Notifications
You must be signed in to change notification settings - Fork 199
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
Update stylings of quiz questions in the submitted state in learning mode #7129
Update stylings of quiz questions in the submitted state in learning mode #7129
Conversation
…f-submitted-quizzes
WordPress Dependencies ReportThe
This comment was automatically generated by the |
…f-submitted-quizzes
Codecov Report
@@ Coverage Diff @@
## feature/frontend-improvements #7129 +/- ##
================================================================
Coverage 49.34% 49.34%
Complexity 10548 10548
================================================================
Files 575 575
Lines 44564 44566 +2
Branches 402 402
================================================================
+ Hits 21988 21989 +1
- Misses 22249 22250 +1
Partials 327 327
Continue to review full report in Codecov by Sentry.
|
I noticed a small issue and added a fix here, wanted to share the finding. I found out that the checked checkboxs looks broken in some quizzes. After some investigation I found out that they look broken only when the quiz has no Multi Line type question. After more investigation to figure out why it’s happening, I found out that the Multi Line uses tinymce editor which we already know, and this tinymce editor enqueues the dashicons styles. The checkmark in the checkbox uses the dashicons font-family. So when we don’t have the tinymce editor loading for any question in the quiz, the checked checkboxes look broken. The solution was simple, I just enqueued the dashicon styles |
Also put properties in alpha order and remove invalid `vertical-align` property.
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.
@imran I made some tweaks for things I found during testing. A few things to watch out for in the future:
- That naming conventions are followed for things like CSS classes. I've addressed this in 095d596.
- That you walk through the Pre-Merge checklist and ensure CI jobs are passing before asking for a review.
- Whenever templates are changed, the version number needs to be bumped and ideally the PHP Templates label applied to the PR. Users overwrite template files sometimes, so updating the version number is one way to communicate that they should update their local versions. I've done that in 20c423b.
- Related to the previous point, we need to add a "development" change log entry for the template changes. This is another way to inform developers to update their templates. That means in this PR, I've added two change log entries. If I've done this correctly, you should see them both show up in the appropriate sections of the change log when it's auto-generated on release day.
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.
Let's go! 🥳
9daba73
into
feature/frontend-improvements
Resolves #7128
Stacked on #7124
Proposed Changes
Testing Instructions
Desktop
Mobile
New/Updated Hooks
sensei_quiz_question_answers_inside_before
- Hook used for printing content inside the question answers section before the template content.sensei_quiz_question_answers_inside_after
- Hook used for printing content inside the question answers section after the template content.Pre-Merge Checklist