-
Notifications
You must be signed in to change notification settings - Fork 40
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
Fix #246: Header must be fetched from jakarta.ee #293
Open
yashTEF
wants to merge
5
commits into
eclipse-ee4j:master
Choose a base branch
from
yashTEF:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
2869655
Fetch Header/Footer Functionality
yashTEF c44903b
Fix Indent
yashTEF ca20f2f
Merge branch 'eclipse-ee4j:master' into master
yashTEF 2440d6a
overwrite header/footer with updated files from remote url
yashTEF 7a58a51
change to debug logs
yashTEF File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
ui/src/main/java/org/eclipse/starter/ui/FetchHtmlUtility.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package org.eclipse.starter.ui; | ||
|
|
||
| import java.io.IOException; | ||
| import java.nio.charset.Charset; | ||
| import java.nio.file.Files; | ||
| import java.nio.file.Paths; | ||
|
|
||
| import javax.enterprise.context.ApplicationScoped; | ||
| import javax.faces.context.FacesContext; | ||
| import javax.inject.Named; | ||
|
|
||
| import org.jsoup.Jsoup; | ||
| import org.jsoup.nodes.Document; | ||
| import org.jsoup.select.Elements; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| @Named | ||
| @ApplicationScoped | ||
| public class FetchHtmlUtility { | ||
|
|
||
| private static final String FOOTER_TEMPLATE = "_template_footer_.html"; | ||
| private static final String HEADER_TEMPLATE = "_template_header_.html"; | ||
|
|
||
| Logger logger = LoggerFactory.getLogger(FetchHtmlUtility.class.getName()); | ||
|
|
||
| public String getHeaderHtml() throws IOException { | ||
| String absoluteDiskPath = FacesContext.getCurrentInstance().getExternalContext().getRealPath(HEADER_TEMPLATE); | ||
| String content = readFile(absoluteDiskPath, Charset.defaultCharset()); | ||
| Document document = Jsoup.parse(content); | ||
| Elements headerFragment = document.select("body"); | ||
| StringBuilder headerHtml = new StringBuilder(); | ||
| headerHtml.append(headerFragment.html()); | ||
| logger.debug("element: {}", headerHtml); | ||
| return headerHtml.toString(); | ||
| } | ||
|
|
||
| public String getFooterHtml() throws IOException { | ||
| String absoluteDiskPath = FacesContext.getCurrentInstance().getExternalContext().getRealPath(FOOTER_TEMPLATE); | ||
| String content = readFile(absoluteDiskPath, Charset.defaultCharset()); | ||
| Document document = Jsoup.parse(content); | ||
| Elements footerFragment = document.select("body"); | ||
| StringBuilder footerHtml = new StringBuilder(); | ||
| footerHtml.append(footerFragment.html()); | ||
| logger.debug("element: {}", footerFragment.html()); | ||
| return footerHtml.toString(); | ||
| } | ||
|
|
||
| private static String readFile(String path, Charset encoding) throws IOException { | ||
| byte[] encoded = Files.readAllBytes(Paths.get(path)); | ||
| return new String(encoded, encoding); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,100 +1,6 @@ | ||
| <ui:composition | ||
| xmlns:ui="http://xmlns.jcp.org/jsf/facelets" xmlns="http://www.w3.org/1999/xhtml"> | ||
| <footer id="solstice-footer"> | ||
| <div class="container"> | ||
| <div class="row"> | ||
| <a class="scrollup" href="#">Back to the top</a> | ||
| <div class="col-sm-24 margin-top-20"> | ||
| <div class="row"> | ||
| <div class="col-md-16" id="copyright"> | ||
| <p id="copyright-text">Copyright © Eclipse Foundation, Inc. All Rights Reserved.</p> | ||
| </div> | ||
| <div class="col-md-8 social-media"> | ||
| <ul class="list-inline"> | ||
| <li> | ||
| <a class="social-media-link fa-stack fa-lg" href="https://twitter.com/JakartaEE"> | ||
| <i class="fa fa-circle-thin fa-stack-2x"></i> | ||
| <i class="fa fa-twitter fa-stack-1x"></i> | ||
| <span class="sr-only">Twitter account</span> | ||
| </a> | ||
| </li> | ||
| <li> | ||
| <a class="social-media-link fa-stack fa-lg" | ||
| href="https://www.facebook.com/JakartaEE"> | ||
| <i class="fa fa-circle-thin fa-stack-2x"></i> | ||
| <i class="fa fa-facebook fa-stack-1x"></i> | ||
| <span class="sr-only">Facebook account</span> | ||
| </a> | ||
| </li> | ||
| <li> | ||
| <a class="social-media-link fa-stack fa-lg" | ||
| href="https://www.youtube.com/channel/UC4M7h5b6elu9VlzjLfzuXyg"> | ||
| <i class="fa fa-circle-thin fa-stack-2x"></i> | ||
| <i class="fa fa-youtube fa-stack-1x"></i> | ||
| <span class="sr-only">Youtube account</span> | ||
| </a> | ||
| </li> | ||
| <li> | ||
| <a class="social-media-link fa-stack fa-lg" | ||
| href="https://www.linkedin.com/groups/13597511/" title=""> | ||
| <i class="fa fa-circle-thin fa-stack-2x"></i> | ||
| <i class="fa fa-linkedin fa-stack-1x"></i> | ||
| <span class="sr-only">Linkedin account</span> | ||
| </a> | ||
| </li> | ||
| </ul> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <section class="col-sm-6 hidden-print" id="footer-useful-links"> | ||
| <h2 class="section-title">Useful Links</h2> | ||
| <ul class="nav"> | ||
| <li><a href="https://bugs.eclipse.org/bugs/">Report a Bug</a></li> | ||
| <li><a href="//help.eclipse.org/">Documentation</a></li> | ||
| <li><a href="https://www.eclipse.org/contribute/">How to Contribute</a></li> | ||
| <li><a href="https://www.eclipse.org/mail/">Mailing Lists</a></li> | ||
| <li><a href="https://www.eclipse.org/forums/">Forums</a></li> | ||
| <li><a href="//marketplace.eclipse.org">Marketplace</a></li> | ||
| </ul> | ||
| </section> | ||
| <section class="col-sm-6 hidden-print" id="footer-other"> | ||
| <h2 class="section-title">Other</h2> | ||
| <ul class="nav"> | ||
| <li><a href="https://www.eclipse.org/ide/">IDE and Tools</a></li> | ||
| <li><a href="https://www.eclipse.org/projects">Projects</a></li> | ||
| <li><a href="https://www.eclipse.org/org/workinggroups/">Working Groups</a></li> | ||
| <li><a href="https://www.eclipse.org/org/research/">Research@Eclipse</a></li> | ||
| <li><a href="https://www.eclipse.org/security/">Report a Vulnerability</a></li> | ||
| <li><a href="https://status.eclipse.org">Service Status</a></li> | ||
| </ul> | ||
| </section> | ||
| <section class="col-sm-6 hidden-print" id="footer-eclipse-foundation"> | ||
| <h2 class="section-title">Eclipse Foundation</h2> | ||
| <ul class="nav"> | ||
| <li><a href="https://www.eclipse.org/org/">About Us</a></li> | ||
| <li><a href="https://www.eclipse.org/org/foundation/contact.php">Contact Us</a></li> | ||
| <li><a href="https://www.eclipse.org/donate">Donate</a></li> | ||
| <li><a href="https://www.eclipse.org/membership">Members</a></li> | ||
| <li><a href="https://www.eclipse.org/org/documents/">Governance</a></li> | ||
| <li><a href="https://www.eclipse.org/org/documents/Community_Code_of_Conduct.php">Code | ||
| of Conduct</a></li> | ||
| <li><a href="https://www.eclipse.org/artwork/">Logo and Artwork</a></li> | ||
| <li><a href="https://www.eclipse.org/org/foundation/directors.php">Board of | ||
| Directors</a></li> | ||
| </ul> | ||
| </section> | ||
| <section class="col-sm-6 hidden-print" id="footer-legal"> | ||
| <h2 class="section-title">Legal</h2> | ||
| <ul class="nav"> | ||
| <li><a href="https://www.eclipse.org/legal/privacy.php">Privacy Policy</a></li> | ||
| <li><a href="https://www.eclipse.org/legal/termsofuse.php">Terms of Use</a></li> | ||
| <li><a href="https://www.eclipse.org/legal/copyright.php">Copyright Agent</a></li> | ||
| <li><a href="https://www.eclipse.org/legal/epl-2.0/">Eclipse Public License</a></li> | ||
| <li><a href="https://www.eclipse.org/legal/">Legal Resources </a></li> | ||
| </ul> | ||
| </section> | ||
| </div> | ||
| </div> | ||
| </footer> | ||
| xmlns:ui="http://xmlns.jcp.org/jsf/facelets" xmlns="http://www.w3.org/1999/xhtml" | ||
| xmlns:h="http://xmlns.jcp.org/jsf/html"> | ||
| <h:outputText value="#{fetchHtmlUtility.footerHtml}" escape="false"/> | ||
| <script src="https://jakarta.ee/js/solstice.js"></script> | ||
| </ui:composition> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why does this need to be done every time? It looks like this is static. Why can't be do this in the Maven build too? Also, why can’t we just directly include the file if it can be valid XHTML?
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 is my first time seeing this PR. My first impression is that using the JSF Facelets
<ui:include>feature is an easier way to accomplish what theFetchHtmlUtilityis doing.At a very high level, using Java code to generate markup is an anti-pattern. A more current (but certainly not the only) way to do it is to keep in XHTML as much as possible and use Expression Language elements in the XHTML to pull in the parts that need to be dynamic.
This is a StackOver Flow question that talks about this. https://stackoverflow.com/questions/8947425/how-to-conditionally-include-a-file-in-my-template-using-jsf-and-facelets
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.
There was some existing XHTML code present in the topbar.xhtml which was needed along with the html code fetched from the url, some parsing along with placing the code inside ui:composition tag made me think of the above approach as it was a bit straightforward but as Reza mentioned I did not take into the consideration the performance issue of doing the above repeatedly.
I'll try to find some out of the box feature present in maven plugins which does the above so that all of this can be done during the build itself.