Skip to content
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

Open
ivargrimstad opened this issue Mar 31, 2023 · 65 comments
Open

Header must be fetched from jakarta.ee #246

ivargrimstad opened this issue Mar 31, 2023 · 65 comments
Assignees
Labels
high-priority High priority issue UI User Interface

Comments

@ivargrimstad
Copy link
Member

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:
starter-INCORRECT

This is how it should be:
jakarta ee-CORRECT

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.

@ivargrimstad ivargrimstad added high-priority High priority issue UI User Interface labels Mar 31, 2023
@m-reza-rahman
Copy link
Contributor

I’ll ask Kito to take care of this. Bazlur made an attempt.

@kito99
Copy link

kito99 commented Mar 31, 2023

@ivargrimstad is it okay to copy it or are you expecting it to be pulled dynamically during the build?

@m-reza-rahman m-reza-rahman assigned kito99 and unassigned rokon12 Mar 31, 2023
@ivargrimstad
Copy link
Member Author

It would be better to include it dynamically, so changes are reflected. Copy would be a quick fix

@m-reza-rahman
Copy link
Contributor

m-reza-rahman commented Mar 31, 2023

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.

@kito99
Copy link

kito99 commented Apr 22, 2023

@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.

@BalusC
Copy link

BalusC commented May 8, 2023

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.

@kito99
Copy link

kito99 commented May 8, 2023

@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.

@ivargrimstad
Copy link
Member Author

@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?

@chrisguindon
Copy link
Member

@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?

@autumnfound
Copy link

@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.

@BalusC
Copy link

BalusC commented May 8, 2023

If it were exposed as an API service, these could be supplied as parameters.

@autumnfound
Copy link

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:

  • {{TITLE}} for page title
  • {{KEYWORDS}} for SEO keywords
  • !!DESCRIPTION!! for page description. Due to a limitation in how we print this content, I needed to use a less unique pattern than above.

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?

@kito99
Copy link

kito99 commented May 11, 2023

@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.

@autumnfound
Copy link

@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 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 https://jakarta.ee/templates/header/ and https://jakarta.ee/templates/footer/. This header would shift once on Jakarta to be just the inner page header for this site. This could be grabbed at build time using curl or wget and thrown into your resources folder to make it available to your web app template. This means we wouldn't need to create a new API for a limited use case but still provide a way to get up-to-date fragments at build time.

@kito99
Copy link

kito99 commented May 19, 2023

@autumnfound ah, okay. That sounds prefect. How do we parameterize the template? URL parameters?

@autumnfound
Copy link

@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.

@kito99
Copy link

kito99 commented Jun 16, 2023

@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.

@kito99
Copy link

kito99 commented Jun 29, 2023

@autumnfound bump 🙂

@autumnfound
Copy link

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.

@autumnfound
Copy link

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

jakartaee/jakarta.ee#1686

@autumnfound
Copy link

Alright, the initial version is now available! See https://jakarta.ee/templates/header/ and https://jakarta.ee/templates/footer/ for the fragments

@kito99
Copy link

kito99 commented Jul 6, 2023

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?

@chrisguindon
Copy link
Member

chrisguindon commented Jul 6, 2023

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 </body> tag.

@chrisguindon
Copy link
Member

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:

<script src="/js/solstice.js?v2.3?v=1688650700)"></script>
<link rel="stylesheet" href="/css/styles.css?v2.3?v=1688650700">

@autumnfound
Copy link

The existing header/footer are also using the absolute path for the images.

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.

@autumnfound
Copy link

The change has been merged, so the logo should now have the proper absolute path!

@kito99
Copy link

kito99 commented Jan 19, 2024

@yashTEF do you need to do any more testing, or are is it ready for a final review?

@yashTEF
Copy link

yashTEF commented Jan 29, 2024

@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.

@kito99
Copy link

kito99 commented Jan 31, 2024

Great, thanks @yashTEF!

@kito99
Copy link

kito99 commented Feb 16, 2024

@yashTEF how is this going?

@yashTEF
Copy link

yashTEF commented Feb 29, 2024

A little blocked with some other work at the moment, will pick this up ASAP.

@kito99
Copy link

kito99 commented Mar 20, 2024

@yashTEF how is it looking for you in the next couple of weeks?

@kito99
Copy link

kito99 commented Apr 15, 2024

@yashTEF just following up..

@yashTEF
Copy link

yashTEF commented Apr 24, 2024

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?

@m-reza-rahman
Copy link
Contributor

Please try and prioritize it, Yash. We are deferring features so the UI revamp work can be sorted with minimum hassle.

@kito99
Copy link

kito99 commented May 8, 2024

@yashTEF will you have time soon? We'd really like to close this.

@yashTEF
Copy link

yashTEF commented May 14, 2024

Yes, have started working on this. Will update here on any progress.

@kito99
Copy link

kito99 commented May 30, 2024

Hey @yashTEF the month is almost over. Do you think you'll be able to complete this soon?

@yashTEF
Copy link

yashTEF commented May 30, 2024

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

@autumnfound
Copy link

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!

@yashTEF
Copy link

yashTEF commented Jun 19, 2024

Hi @autumnfound , I noticed when fetching the footer from the Jakarta.ee endpoint the
html fragment that I am receiving in response does not have the opening tags as well as the html code for some of the elements for e.g. html, body tags don't have an opening tag.

For inspecting I tried a curl request and viewed the response, Is there any particular reason for this?

@m-reza-rahman
Copy link
Contributor

For Faces in particular, valid XHTML would be far preferable.

@autumnfound
Copy link

For inspecting I tried a curl request and viewed the response, Is there any particular reason for this?

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.

For Faces in particular, valid XHTML would be far preferable.

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?

@kito99
Copy link

kito99 commented Jun 19, 2024

Another, less "proper" approach would be to just strip out the and sections from the content returned for the footer.

@autumnfound
Copy link

autumnfound commented Jun 20, 2024

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!

@kito99
Copy link

kito99 commented Jun 20, 2024

@autumnfound no, I'm saying you leave it as-is, and we strip out the offending end tags on our end.

@yashTEF
Copy link

yashTEF commented Jun 22, 2024

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?

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.

@autumnfound
Copy link

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.

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.

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.

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 div#content-placeholder, and you can see the base theme version introduced by the PR on this page: https://preview-371--webdev-eclipse-org-docs-hugo.eclipsecontent.org/docs/hugo/templates/page/

@autumnfound
Copy link

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

@autumnfound
Copy link

@yashTEF bumping thread, does the above work, or should we look into something else?

@yashTEF
Copy link

yashTEF commented Jul 17, 2024

@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?

@autumnfound
Copy link

autumnfound commented Jul 17, 2024

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.

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>

@autumnfound
Copy link

@yashTEF @kito99 if the mixed implementation I wrote up in pseudo code works better than the other 2 proposed, let me know so I can implement the needed fragments! I'm just waiting on feedback to implement and merge at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority High priority issue UI User Interface
Projects
None yet
Development

No branches or pull requests

8 participants