Synchrnous PI fixes from feedback#1228
Conversation
|
When we build the nginx container we copy only the components/rsptx/templates/staticAssets folder into the nginx container. It does not have access to the web2py files. |
bnmnetp
left a comment
bnmnetp
left a comment
There was a problem hiding this comment.
It does not look like this fixes any routing problems. Can you make a list of where you think the routing has gone wrong?
| } | ||
| doIt(); | ||
|
|
||
| setTimeout(function addLineNumbers() { |
There was a problem hiding this comment.
Is this trying to add line numbers to code in a multiple choice question? If the question is written properly it will already have line numbers.
There was a problem hiding this comment.
Yeah this is adding in line numbers that are not rendering during PI even when the question is created with them. I believe the questions are being created properly using the assignment builder interface and it still does not show them.
There was a problem hiding this comment.
Can you point me to a specific question? It may be that we are just missing including the prism js/css files that do the coloring and line numbers.
There was a problem hiding this comment.
Yes, the templates for peer_question.html and doAssignment.html are missing the needed prism files for decorating code questions. Adding those is the right fix.
|
For the routing issue, when navigating to PI from the book page, it routes to |
|
Yes we need to make some updates to the master templates of the books both on the |
|
Ok I went ahead and fixed the line numbers by adding the missing Prism CSS to static_assets.html and removing the custom JS workaround I had. It works for questions created with the new assignment builder but older questions don't show line numbers since their HTML source is missing the line-numbers class on the pre tag. Is it worth backfilling that class on existing questions or is the expectation that instructors should just update them? |
|
When you say old questions, are they questions written in PreTeXt or are they old RST based questions? Old RST questions required that the author used |
|
Old RST-based questions. They use div class="highlight-python notranslate" instead of pre class="line-numbers" so Prism doesn't pick them up. |
|
No RST used its own method for adding line numbers and coloring code. |
|
Se my little edit to the previous comment. |
|
I see then so for those RST questions without line numbers it was just an authoring choice. In that case is it best to leave those as is? |
|
Yeah, if the authors of those questions want line numbers they can edit the book (or question) and add them. I don't think we should impose a style over the top of something an author already has control of. |
|
Now, time is short for RST questions. The editor for those old questions will not be gone by the start of classes in the fall. |
|
Got it, makes sense to leave it to the authors then. I also cleaned up some leftover line number CSS and JS from the dashboard template. Anything else I should do before merge? |
There was a problem hiding this comment.
Pull request overview
This PR updates the Peer Instruction (PI) synchronous workflow and UI to address issues found during testing/instructor feedback, including clearer student guidance, improved group-selection UX, and better vote-count visibility.
Changes:
- Adjust PI messaging to avoid prompting discussion prematurely and simplify the “verbal discussion group” instructions.
- Enhance in-person group selection UX (clearer instructions + Select2 placeholder tweaks).
- Add Prism line-number styling assets to support line-numbered code blocks during PI.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| components/rsptx/templates/staticAssets/js/peer.js | Updates PI control-message text, simplifies face-chat messaging, and copies Vote 1 count into a new UI element when starting Vote 2; tweaks Select2 placeholder. |
| components/rsptx/templates/common/static_assets.html | Adds Prism theme + line-numbers plugin CSS to visually enable code line numbers. |
| components/rsptx/templates/assignment/student/peer_question.html | Improves group-selection instructions and adds an empty option to support placeholder behavior. |
| components/rsptx/templates/assignment/instructor/peer_dashboard.html | Adds UI elements to display Vote 1 count during Vote 2; removes an older session-status element. |
| bases/rsptx/assignment_server_api/routers/peer.py | Removes instructor-only restriction from /course_students endpoint (now returns {username: full_name} for the course). |
Comments suppressed due to low confidence (1)
bases/rsptx/assignment_server_api/routers/peer.py:1230
- Removing @instructor_role_required() makes this endpoint available to any logged-in user in the course and returns a full roster mapping of username -> full name. Elsewhere (/ns/auth/course_students) roster access is restricted for base courses and instructors are filtered out; aligning this endpoint with that behavior would reduce unintended data exposure.
@router.get("/course_students")
@with_course()
async def get_course_students(
request: Request,
user=Depends(auth_manager),
course=None,
):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Fixed several sync PI issues from testing feedback:
Also, some PI pages still seem to route to web2py instead of FastAPI. I think it's because nginx serves /staticAssets/ from the web2py static directory, but if it's something else on my end let me know and I can fix it.