SAK-52316 Samigo numeric response tooltip requires bootstrap#14366
SAK-52316 Samigo numeric response tooltip requires bootstrap#14366ern wants to merge 1 commit intosakaiproject:masterfrom
Conversation
WalkthroughA JSP file's script loading mechanism is refactored. The direct qtip2 WebJar inclusion is removed and replaced with an external script reference to headscripts.js and a new initialization block that loads jQuery, qtip2, and bootstrap. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@samigo/samigo-app/src/webapp/jsf/delivery/item/deliverFillInNumeric.jsp`:
- Around line 27-32: The explicit script tag loading
"/library/js/headscripts.js" should be removed because parent pages inject it
via html.head; also remove the redundant includeWebjarLibrary('bootstrap') call
since deliverAssessment.jsp already provides Bootstrap; keep only
includeLatestJQuery('deliverFillInNumeric') and includeWebjarLibrary('qtip2') in
this fragment (references: the literal script tag for headscripts.js,
includeLatestJQuery('deliverFillInNumeric'), includeWebjarLibrary('qtip2') and
includeWebjarLibrary('bootstrap')).
🧹 Nitpick comments (1)
samigo/samigo-app/src/webapp/jsf/delivery/item/deliverFillInNumeric.jsp (1)
29-31: Consider migrating from qtip2 to Bootstrap 5 native tooltips as a follow-up task.The deliverFillInNumeric.jsp uses
hasTooltipandtoolTipLinkclasses that rely on qtip2. Since Bootstrap 5.2.0 (the configured version in the project's webjar configuration) ships with native Tooltip component usingdata-bs-toggle="tooltip", removing qtip2 in favor of Bootstrap's built-in tooltips would reduce the dependency surface and align with the coding guidelines to leverage Bootstrap 5 components.
| <script src="/library/js/headscripts.js"></script> | ||
| <script> | ||
| includeLatestJQuery('deliverFillInNumeric'); | ||
| includeWebjarLibrary('qtip2'); | ||
| includeWebjarLibrary('bootstrap'); | ||
| </script> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find which files include deliverFillInNumeric.jsp
echo "=== Files that include deliverFillInNumeric.jsp ==="
rg -l "deliverFillInNumeric" --type jsp 2>/dev/null || rg "deliverFillInNumeric" -l -g "*.jsp" 2>/dev/null
echo ""
echo "=== Check if parent delivery pages load headscripts.js ==="
rg -n "headscripts" -g "*.jsp" samigo/samigo-app/src/webapp/jsf/delivery/ 2>/dev/null | head -30
echo ""
echo "=== Check other item include files for similar patterns ==="
ls -la samigo/samigo-app/src/webapp/jsf/delivery/item/*.jsp | head -10
echo ""
echo "=== Check headscripts loading in item folder ==="
rg -n "headscripts" samigo/samigo-app/src/webapp/jsf/delivery/item/ 2>/dev/nullRepository: sakaiproject/sakai
Length of output: 2020
🏁 Script executed:
#!/bin/bash
# Check deliverAssessment.jsp for headscripts loading
echo "=== Check deliverAssessment.jsp content ==="
head -50 samigo/samigo-app/src/webapp/jsf/delivery/deliverAssessment.jsp
echo ""
echo "=== Check if deliverAssessment.jsp loads any scripts ==="
rg -n "script|includeWebjarLibrary|includeLatestJQuery" samigo/samigo-app/src/webapp/jsf/delivery/deliverAssessment.jsp | head -20
echo ""
echo "=== Check other item includes for includeWebjarLibrary calls ==="
rg -n "includeWebjarLibrary" samigo/samigo-app/src/webapp/jsf/delivery/item/
echo ""
echo "=== Check Bootstrap version in pom.xml ==="
rg -n "bootstrap" samigo/samigo-app/pom.xml | grep -i "version\|<version"Repository: sakaiproject/sakai
Length of output: 4693
🏁 Script executed:
#!/bin/bash
# Check deliveryjQuery.jsp
echo "=== Content of deliveryjQuery.jsp ==="
cat samigo/samigo-app/src/webapp/jsf/delivery/deliveryjQuery.jsp
echo ""
echo "=== Check deliverAudioRecording.jsp for headscripts loading ==="
head -40 samigo/samigo-app/src/webapp/jsf/delivery/item/deliverAudioRecording.jsp
echo ""
echo "=== Search for includeWebjarLibrary definition ==="
rg -n "function includeWebjarLibrary|includeWebjarLibrary\s*=" --type=jsp -A 5 | head -30Repository: sakaiproject/sakai
Length of output: 7612
🏁 Script executed:
#!/bin/bash
# Search for headscripts.js definition and includeWebjarLibrary
echo "=== Find headscripts.js ==="
find . -name "headscripts.js" -type f 2>/dev/null
echo ""
echo "=== Check if headscripts.js is loaded in portal header ==="
rg "headscripts" --type=js --type=jsp -B 2 -A 2 | head -40
echo ""
echo "=== Check if deliverAssessment.jsp uses request.getAttribute for html.head ==="
grep -n "html.head" samigo/samigo-app/src/webapp/jsf/delivery/deliverAssessment.jsp
echo ""
echo "=== Check if other parent pages that include this file also load bootstrap ==="
rg -n "bootstrap" samigo/samigo-app/src/webapp/jsf/evaluation/gradeStudentResult.jsp samigo/samigo-app/src/webapp/jsf/delivery/reviewAssessment.jsp 2>/dev/nullRepository: sakaiproject/sakai
Length of output: 449
🏁 Script executed:
#!/bin/bash
# Check if deliveryjQuery.jsp loads headscripts.js
echo "=== Full search for headscripts in JSP files ==="
grep -r "headscripts" samigo/samigo-app/src/webapp/jsf/ 2>/dev/null
echo ""
echo "=== Check library headscripts.js to understand includeWebjarLibrary ==="
head -100 ./library/src/webapp/js/headscripts.js
echo ""
echo "=== Check parent pages: gradeStudentResult.jsp ==="
head -60 samigo/samigo-app/src/webapp/jsf/evaluation/gradeStudentResult.jsp | grep -E "head|script|include"
echo ""
echo "=== Check parent pages: reviewAssessment.jsp ==="
head -60 samigo/samigo-app/src/webapp/jsf/delivery/reviewAssessment.jsp | grep -E "head|script|include"Repository: sakaiproject/sakai
Length of output: 4321
Remove the explicit <script src="/library/js/headscripts.js"></script> tag; parent pages already provide it via the portal.
This JSP is an include fragment included by deliverAssessment.jsp, reviewAssessment.jsp, and gradeStudentResult.jsp. All parent pages inject headscripts.js and other portal resources via <%= request.getAttribute("html.head") %>, making the explicit script tag at line 27 redundant. Other item includes like deliverAudioRecording.jsp rely on this same pattern without loading headscripts.js directly.
Additionally, deliverAssessment.jsp already loads bootstrap/5.2.0 directly (line 44), so the includeWebjarLibrary('bootstrap') call at line 31 is unnecessary. Keep only includeLatestJQuery('deliverFillInNumeric') and includeWebjarLibrary('qtip2').
🤖 Prompt for AI Agents
In `@samigo/samigo-app/src/webapp/jsf/delivery/item/deliverFillInNumeric.jsp`
around lines 27 - 32, The explicit script tag loading
"/library/js/headscripts.js" should be removed because parent pages inject it
via html.head; also remove the redundant includeWebjarLibrary('bootstrap') call
since deliverAssessment.jsp already provides Bootstrap; keep only
includeLatestJQuery('deliverFillInNumeric') and includeWebjarLibrary('qtip2') in
this fragment (references: the literal script tag for headscripts.js,
includeLatestJQuery('deliverFillInNumeric'), includeWebjarLibrary('qtip2') and
includeWebjarLibrary('bootstrap')).
1 participant
https://sakaiproject.atlassian.net/browse/SAK-52316
Summary by CodeRabbit