-
Notifications
You must be signed in to change notification settings - Fork 6
feat: switch from angular/AJAX+Boostrap to HTMX+PicoCSS #345
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -266,42 +266,54 @@ public void queryRequirementsAsHtml( | |||||||
| ) | ||||||||
| @GET | ||||||||
| @Path("selector") | ||||||||
| @Consumes({ MediaType.TEXT_HTML, MediaType.WILDCARD }) | ||||||||
| @Consumes({MediaType.TEXT_HTML, MediaType.WILDCARD}) | ||||||||
| public Response RequirementSelector( | ||||||||
| @QueryParam("terms") final String terms | ||||||||
| , @PathParam("serviceProviderId") final String serviceProviderId | ||||||||
| ) throws ServletException, IOException, JSONException | ||||||||
| { | ||||||||
| ) throws ServletException, IOException, JSONException { | ||||||||
| // Start of user code RequirementSelector_init | ||||||||
| // End of user code | ||||||||
|
|
||||||||
| httpServletRequest.setAttribute("selectionUri",UriBuilder.fromUri(OSLC4JUtils.getServletURI()).path(uriInfo.getPath()).build().toString()); | ||||||||
| // End of user code | ||||||||
|
|
||||||||
| httpServletRequest.setAttribute("selectionUri", | ||||||||
| UriBuilder.fromUri(OSLC4JUtils.getServletURI()).path(uriInfo.getPath()).build().toString()); | ||||||||
| // Start of user code RequirementSelector_setAttributes | ||||||||
| // End of user code | ||||||||
|
|
||||||||
| if (terms != null ) { | ||||||||
| // End of user code | ||||||||
| if (terms != null) { | ||||||||
| httpServletRequest.setAttribute("terms", terms); | ||||||||
| final List<Requirement> resources = delegate.RequirementSelector(httpServletRequest, serviceProviderId, terms); | ||||||||
| if (resources!= null) { | ||||||||
| JSONArray resourceArray = new JSONArray(); | ||||||||
| for (Requirement resource : resources) { | ||||||||
| JSONObject r = new JSONObject(); | ||||||||
| r.put("oslc:label", resource.toString()); | ||||||||
| r.put("rdf:resource", resource.getAbout().toString()); | ||||||||
| r.put("Label", resource.toString()); | ||||||||
| // Start of user code RequirementSelector_setResponse | ||||||||
| // End of user code | ||||||||
| resourceArray.add(r); | ||||||||
| final List<Requirement> resources = delegate.RequirementSelector(httpServletRequest, | ||||||||
| serviceProviderId, terms); | ||||||||
| if (resources != null) { | ||||||||
| if ("true".equalsIgnoreCase(httpServletRequest.getHeader("hx-request"))) { | ||||||||
| // we need to return HTML fragment | ||||||||
| RequestDispatcher rd = httpServletRequest.getRequestDispatcher("/co/oslc/refimpl/rm" + | ||||||||
| "/gen/requirementselector-results.jsp"); | ||||||||
| httpServletRequest.setAttribute("resources", resources); | ||||||||
|
||||||||
| httpServletRequest.setAttribute("resources", resources); | |
| httpServletRequest.setAttribute("resources", resources); |
Copilot
AI
Dec 13, 2025
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.
When forwarding the request to a JSP that returns HTML fragments for HTMX (lines 289-292), the method continues execution after rd.forward() without returning. This means the subsequent code block (lines 310-313) will still be evaluated, potentially causing issues. Add a return statement after the forward() call to prevent fall-through.
| rd.forward(httpServletRequest, httpServletResponse); | |
| rd.forward(httpServletRequest, httpServletResponse); | |
| return null; |
Copilot
AI
Dec 13, 2025
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.
The comment on lines 294-295 acknowledges that mixing HTML and JSON responses without proper content negotiation is "really bad", but this approach is still implemented. This creates technical debt and violates REST API principles. Consider refactoring to use proper Accept header negotiation or creating separate endpoints for HTML and JSON responses.
Copilot
AI
Dec 13, 2025
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.
When the delegate returns null for resources, the code logs an error stating "A empty search should return an empty list and not NULL!" and then returns a 204 No Content response. However, this masks a potential bug in the delegate implementation rather than handling it appropriately. Consider throwing an exception or returning a 500 Internal Server Error to signal that the delegate violated its contract.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,34 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <%-- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Copyright (c) 2025 Contributors to the Eclipse Foundation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| See the NOTICE file(s) distributed with this work for additional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| information regarding copyright ownership. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This program and the accompanying materials are made available under the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| terms of the Eclipse Distribution License 1.0 which is available at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| http://www.eclipse.org/org/documents/edl-v10.php. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SPDX-License-Identifier: BSD-3-Simple | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This file is generated by Lyo Designer (https://www.eclipse.org/lyo/) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| --%> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <%@ taglib uri="http://java.sun.com/jsp/jstl/functions" prefix="fn" %> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <%@page import="org.eclipse.lyo.oslc.domains.rm.Requirement"%> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <%@ page contentType="text/html" language="java" pageEncoding="UTF-8" %> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <aside> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <nav> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <ul> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <c:forEach var="requirement" items="${resources}"> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <li> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <a href="${requirement.about}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| onclick="sendOslcSelectionPostMessage(this, event)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| >${requirement.title}</a> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+27
to
+29
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| <a href="${requirement.about}" | |
| onclick="sendOslcSelectionPostMessage(this, event)" | |
| >${requirement.title}</a> | |
| <a href="<c:out value='${requirement.about}'/>" | |
| onclick="sendOslcSelectionPostMessage(this, event)" | |
| ><c:out value="${requirement.title}"/></a> |
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.
this tiny bit of HTML is what gets rendered every time HTMX makes a search request and it dynamically replaces the contents of the #search-results div with this content returned by the server. Simple and easy.
Copilot
AI
Dec 13, 2025
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.
The inline JavaScript function 'sendOslcSelectionPostMessage' is referenced in the onclick handler of links in the results JSP, but this creates a tight coupling between the two files. If the results JSP is loaded as an HTML fragment via HTMX, the function needs to exist in the parent page's scope. Consider using event delegation or HTMX events instead of inline onclick handlers for better separation of concerns.
| onclick="sendOslcSelectionPostMessage(this, event)" | |
| >${requirement.title}</a> | |
| </li> | |
| </c:forEach> | |
| </ul> | |
| </nav> | |
| </aside> | |
| >${requirement.title}</a> | |
| </li> | |
| </c:forEach> | |
| </ul> | |
| </nav> | |
| </aside> | |
| <script> | |
| // Event delegation for requirement links | |
| document.addEventListener('DOMContentLoaded', function() { | |
| var nav = document.querySelector('aside nav'); | |
| if (nav) { | |
| nav.addEventListener('click', function(event) { | |
| var target = event.target; | |
| // Traverse up in case of nested elements inside <a> | |
| while (target && target !== nav && !(target.tagName && target.tagName.toLowerCase() === 'a')) { | |
| target = target.parentElement; | |
| } | |
| if (target && target.tagName && target.tagName.toLowerCase() === 'a') { | |
| if (typeof sendOslcSelectionPostMessage === 'function') { | |
| sendOslcSelectionPostMessage(target, event); | |
| event.preventDefault(); | |
| } | |
| } | |
| }); | |
| } | |
| }); | |
| </script> |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,7 +1,7 @@ | ||||||||||||||||||||||||||||||||||
| <%--To avoid the overriding of any manual code changes upon subsequent code generations, disable "Generate JSP Files" option in the Adaptor model.--%> | ||||||||||||||||||||||||||||||||||
| <!DOCTYPE html> | ||||||||||||||||||||||||||||||||||
| <%-- | ||||||||||||||||||||||||||||||||||
| Copyright (c) 2020 Contributors to the Eclipse Foundation | ||||||||||||||||||||||||||||||||||
| Copyright (c) 2025 Contributors to the Eclipse Foundation | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| See the NOTICE file(s) distributed with this work for additional | ||||||||||||||||||||||||||||||||||
| information regarding copyright ownership. | ||||||||||||||||||||||||||||||||||
|
|
@@ -27,38 +27,55 @@ | |||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| <% | ||||||||||||||||||||||||||||||||||
| String selectionUri = (String) request.getAttribute("selectionUri"); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| %> | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| <html> | ||||||||||||||||||||||||||||||||||
| <head> | ||||||||||||||||||||||||||||||||||
| <meta http-equiv="Content-Type" content="text/html;charset=utf-8"> | ||||||||||||||||||||||||||||||||||
| <!DOCTYPE html> | ||||||||||||||||||||||||||||||||||
| <html lang="en"> | ||||||||||||||||||||||||||||||||||
| <head> | ||||||||||||||||||||||||||||||||||
| <meta charset="UTF-8"> | ||||||||||||||||||||||||||||||||||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||||||||||||||||||||||||||||||||||
| <title>RequirementSD</title> | ||||||||||||||||||||||||||||||||||
| <script src="<c:url value="/static/js/delegated-ui.js"/>"></script> | ||||||||||||||||||||||||||||||||||
| </head> | ||||||||||||||||||||||||||||||||||
| <body style="padding: 10px;"> | ||||||||||||||||||||||||||||||||||
| <div id="selector-body"> | ||||||||||||||||||||||||||||||||||
| <p id="searchMessage">Find a specific resource through a free-text search.</p> | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| <p id="loadingMessage" style="display: none;">Pondering your search. Please stand by ...</p> | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@1/css/pico.min.css"> | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@1/css/pico.min.css"> | |
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@1/css/pico.min.css" integrity="sha384-+1Qw6QwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQwQw==" crossorigin="anonymous"> |
Copilot
AI
Dec 13, 2025
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.
The HTMX trigger configuration 'input changed delay:120ms, keyup[key=='Enter'], load' includes a 'load' event which will fire when the page loads, even when the search input is empty. This will cause an unnecessary request to the server on page load. Consider removing 'load' from the trigger if you don't want to perform a search with empty terms on initial page load.
| hx-trigger="input changed delay:120ms, keyup[key=='Enter'], load" | |
| hx-trigger="input changed delay:120ms, keyup[key=='Enter']" |
Copilot
AI
Dec 13, 2025
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.
The wildcard origin '*' in postMessage allows any website to receive the OSLC selection response, which could expose sensitive resource information to malicious sites. Consider using the opener's origin or a configured whitelist of allowed origins instead.
| window.parent.postMessage("oslc-response:" + JSON.stringify(message), '*') | |
| // Extract parent origin from document.referrer | |
| let parentOrigin = null; | |
| try { | |
| if (document.referrer) { | |
| parentOrigin = new URL(document.referrer).origin; | |
| } | |
| } catch (e) { | |
| // If referrer is malformed, do not send the message | |
| parentOrigin = null; | |
| } | |
| if (parentOrigin) { | |
| window.parent.postMessage("oslc-response:" + JSON.stringify(message), parentOrigin); | |
| } else { | |
| console.error("Unable to determine parent origin. OSLC selection response not sent."); | |
| } |
Copilot
AI
Dec 13, 2025
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.
The JavaScript variables 'elt', 'name', and 'evt' use inconsistent naming conventions. The code uses 'elt' (abbreviated) alongside 'name' and 'evt' (also abbreviated). Consider using more descriptive names like 'element', 'eventName', and 'event' for better code clarity, or at least be consistent with abbreviation style.
| onEvent : function(name, evt) { | |
| var elt = evt.detail.elt; | |
| if(name === "htmx:beforeRequest") { | |
| elt.setAttribute("aria-busy", "true") | |
| } else if(name === "htmx:afterRequest" ) { | |
| elt.removeAttribute("aria-busy") | |
| onEvent : function(eventName, event) { | |
| var element = event.detail.elt; | |
| if(eventName === "htmx:beforeRequest") { | |
| element.setAttribute("aria-busy", "true") | |
| } else if(eventName === "htmx:afterRequest" ) { | |
| element.removeAttribute("aria-busy") |
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.
The HTMX request detection using a custom header "hx-request" should be case-sensitive checked. The equalsIgnoreCase method could match "TRUE", "True", etc., but HTMX sends lowercase "true". While this is more lenient (which may be desirable), it's worth noting that the official HTMX header value is always lowercase "true".