-
Notifications
You must be signed in to change notification settings - Fork 42
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
Header must be fetched from jakarta.ee #246
Comments
I’ll ask Kito to take care of this. Bazlur made an attempt. |
@ivargrimstad is it okay to copy it or are you expecting it to be pulled dynamically during the build? |
It would be better to include it dynamically, so changes are reflected. Copy would be a quick fix |
There should be a job setup shortly, so if we want to do something at build time, it should be doable. @jeyvison can help you with the setup. |
@ivargrimstad correct me if I'm wrong, but wouldn't we need to create a separate yarn run script that builds the header by itself? https://github.com/jakartaee/jakarta.ee/blob/src/layouts/partials/header.html. Since this is using Hugo, there is no static version of the header we can just include. |
Public instructions how to reuse the jakarta.ee header from its original source into any random website would be helpful. A dedicated API service which returns the desired raw HTML would be absolutely fabulous. |
@ivargrimstad is there someone who's familiar with the web site build (and hugo) that can help us out with this? As @BalusC mentioned, it's really not clear how to import it dynamically. We can always cut and paste from the generated site, but that is of course prone to long term issues. |
@chrisguindon Do you think someone in webdev could help with this? |
@ivargrimstad We might be able to help. Is it correct for me to assume that need a url/page from jakarta.ee website that only includes the HTML for the footer and header? @autumnfound can you help here? |
I can poke around and see about making some raw HTML fragments that can be generated per site, but that comes with a caveat. There would be no real way to customize the metadata of the page easily, as it would go through the static generator. I could add placeholders that are easy to identify at least, but you'd need to use string substitution to replace those. This is a limitation of the unique combination of systems of automated export of this, and the fact that our sites are statically rendered. |
If it were exposed as an API service, these could be supplied as parameters. |
There are no current plans to implement this as an API, but there is a different solution we can offer that help resolve this need. I've created a patch to our theme that will allow for sites that add the required content to supply HTML fragments for the header and footer. These are pretty simple fragments that include the header w/o breadcrumbs, and the full standard footer. For the header fragment (preview from the PR), I've included some easily identified placeholders to make basic customization possible:
For the footer fragment (preview from the PR), there are no default placeholders at the moment, though I can include a placeholder for importing custom JS or other assets at the bottom of the document. Does this cover what you would need to get started? |
@autumnfound thanks for making this change! I don't quite see how it helps our use case, though (I could be missing something). We basically just want to fetch the complete rendered header from https://github.com/jakartaee/jakarta.ee as a fragment. This project is deployed as as a web app, so it needs to include the header at build time. |
This change would make it so you could grab the header and footer fragments of the Jakarta site once enabled through |
@autumnfound ah, okay. That sounds prefect. How do we parameterize the template? URL parameters? |
You'd have to use string replace for the placeholders I mentioned above. As this is a static site, there is no way to parameterize this, unfortunately. |
@autumnfound ok, now I get it. this would work perfectly! however, I think we'd need this for the jakarta.ee site rather than the eclipse.org site. |
@autumnfound bump 🙂 |
Apologies, we had our annual all hands last week and this slipped attention. We need to update the theme in the main site, and then I can get the template up for Jakarta.ee. I can look into doing it next week as there are some holidays coming up. |
Just to let you guys know, the patch work has been started, but we uncovered some stuff we want to move to our shared front end code, so it's paused while we get that done and published. We're aiming to get this out this week if possible still though |
Alright, the initial version is now available! See https://jakarta.ee/templates/header/ and https://jakarta.ee/templates/footer/ for the fragments |
Thanks @autumnfound! This is just what we need. We can just grab this during the build process. I noticed the menu links don't work; is this something you're planning to work on? |
The header links should work if you have both the header and footer on the same page. The required .js file is right before the closing |
However, I am noticing that the CSS and js files paths don't include the full domain. It will need to point to jakarta.ee/ for them to work outside of the jakarta.ee domain:
|
This looks like this is a problem specific to this site. I'm submitting a patch for review that should add the host to the URL as there is no need for relative URLs here from what I can tell. |
The change has been merged, so the logo should now have the proper absolute path! |
@yashTEF do you need to do any more testing, or are is it ready for a final review? |
I am working on submitting a PR for fixing the links issue in the header/footer templates to https://github.com/jakartaee/jakarta.ee. Also the introduction of the new header has displaced the background a bit so will add a commit for that as well, once this is done, it'll be ready for a review. |
Great, thanks @yashTEF! |
@yashTEF how is this going? |
A little blocked with some other work at the moment, will pick this up ASAP. |
@yashTEF how is it looking for you in the next couple of weeks? |
@yashTEF just following up.. |
Apologies for the delay, haven't logged in for a while. Seems unlikely that I'll be able to complete this at least until mid next month, any chance that this can be pushed back until then? |
Please try and prioritize it, Yash. We are deferring features so the UI revamp work can be sorted with minimum hassle. |
@yashTEF will you have time soon? We'd really like to close this. |
Yes, have started working on this. Will update here on any progress. |
Hey @yashTEF the month is almost over. Do you think you'll be able to complete this soon? |
Yeah, I have opened a PR: jakartaee/jakarta.ee#1925 which needs to be merged. Once that is done I think the outstanding PR will be ready for review |
We've now merged that change, I hadn't seen the email for the updated review. Should be live in the next 5 minutes! |
Hi @autumnfound , I noticed when fetching the footer from the Jakarta.ee endpoint the For inspecting I tried a curl request and viewed the response, Is there any particular reason for this? |
For Faces in particular, valid XHTML would be far preferable. |
When it was generated, this was stated as the header and footer of the page that come together to create the base Jakarta page. When combined, the final output would be valid HTML. The intention is that it could be easier to do the string replaces for the placeholders in the header, and attempts to make it less complicated for integration.
If it would make it easier, we can look at creating a single HTML document that would be the full page, where the body content would need to be injected into the DOM manually. Should we start tracking a request for that change? |
Another, less "proper" approach would be to just strip out the and sections from the content returned for the footer. |
If you are saying we can include the footer and header sections as valid HTML, this gets really sticky as the content doesn't quite split cleanly, and would likely just introduce more files that would make this even worse to maintain. EDIT: accidentally double posted some responses, apologies! |
@autumnfound no, I'm saying you leave it as-is, and we strip out the offending end tags on our end. |
If it makes easier I am able to convert HTML fetched from the url and parse them into well formed XHTML during the build itself using maven plugins. This works well for the header fragment, but for the footer it removes head and body tags as they don't have matching opening tags, if that can be resolved I should be able to convert the footer HTML to valid XHTML like in the case of header. |
If we wanted to use the same "stitching" solution where we give parts of the page to stitch into a larger template, we'd likely have to introduce another 2-3 page fragments at least to get this solution closer to fully functional. There would still likely be some sections where you'd have to hardcode some HTML which would lead to more brittle code, as it would rely on nothing in our styling/page structure changing. Relying on a static structure isn't necessarily the safest thing as it may lead to sudden breaks since you aren't using a static version of the site.
I've created a PR to add the "full page" template to our base theme. This will need to be reviewed, released, and updated on the Jakarta site, but I believe this will likely be a closer solution to what you need. The change here is that you'll need to update the document model to inject the content into the page. The current query to target will be |
If the above is what you need, let me know and I can start the process of getting this page on the Jakarta EE site. If it doesn't we'll need to figure out something else, as I'm not familiar with Faces as I work with a framework that uses a different templating engine which has it's own quirks |
@yashTEF bumping thread, does the above work, or should we look into something else? |
I am trying to understand the change, how can I use the above to fix the problem? From what I can understand there would be a single page template instead of header/footer fragments and the page content will be added by manipulating DOM. @kito99 , does this help or we'll have to change the approach a bit from our side? |
Yep, that's correct! This would eliminate the problem where you have incomplete xhtml fragments which led to some truly strange behaviour where nodes would generate opening tags and lead to bad injection. The only other solution I can think of would have something along the lines of the following (which would require a different change), and isn't ideal as it would lead to issues if we ever changed the HTML around the main content, and would still require DOM manipulation to fix the title in the metadata of the page in the header section (excuse any pseudo code, I don't use xhtml haha): <html>
<xhtml:import uri=".../templates/header" />
<body>
<xhtml:import uri=".../templates/content-preamble" />
<main>
<div class="container padding-bottom-30">
<div class="row">
<div class="col-md-18">
<div class="default-breadcrumbs hidden-print" id="breadcrumb">
<div>
<div class="row">
<div class="col-sm-24">
<ol aria-label="Breadcrumb" class="breadcrumb">
<li><a href="...">Home</a></li>
... (contextual breadcrumbs here)
<li class="active" aria-current="page"><a href="...">{{TITLE}}</a></li>
</ol>
</div>
</div>
</div>
</div>
<div id="content-placeholder">
...
</div>
</div>
</div>
</div>
</main>
<xhtml:import uri=".../templates/footer" />
</body>
</html> |
The header (with menu), as well as the footer, must be synced with the one at https://jakarta.ee
This is the Starter header today:
This is how it should be:
The sources for the https://jakarta.ee website can be found here: https://github.com/jakartaee/jakarta.ee
It is built with Hugo, but should it should be possible to include the header and footer in Faces.
The text was updated successfully, but these errors were encountered: