-
Notifications
You must be signed in to change notification settings - Fork 493
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
Upgrade to PrimeFaces 7 #5975
Comments
@mheppler - does this upgrade include Bootstrap 4 or will that be a different effort? |
UPDATE: It appears that the jQuery requirements of our last upgrade efforts would not effect us for this upgrade. With that last upgrade, we bumped up to jQuery 3, and that was the major hurdle. Looking over the migration guides for both, we should not be blocked to upgrade these independently. It is worth pointing out in this discussion that Bootstrap 4 is considered a "major rewrite of the entire project". |
With #6035 and #5977 prioritized and moving through development, anything of note related to primefaces/primefaces#4849 (h/t @PaulBoon) should be relevant to this issue. Which might mean waiting for the release of PrimeFaces 7.1. |
Still no 7.1... |
Did a quick switch over to
|
Reviewed at tech hours. The two major sources of errors seemed to be: • from org.primefaces.push, which we don't actually use, so is just code that should be easily removable *See https://www.primefaces.org/docs/api/6.2/org/primefaces/context/RequestContext.html And the reference fot the new object: There also was a Base64 issue - it looks like we are using the a primefaces class for encoding to a string in https://github.com/IQSS/dataverse/blob/develop/src/main/java/edu/harvard/iq/dataverse/dataaccess/ImageThumbConverter.java (line 531) and we can just change that to something like java.util.Base64.getEncoder().encodetoString(). |
I got it to the point where it builds and deploys. I obviously haven't verified if all the UI pages are working properly... But basic functionality appears to be there. |
@landreev confirmed a successful build and deploy locally. Clicked around and found everything looks kosher, specifically with the popups and ingest -- which were the code changes related to the compile errors above. I would suggest that instead of commenting all the old code out, that those lines are deleted. GitHub will save previous code for us in their commit history. Also, found a couple of bugs, but was able to replicate them on demo, so I will open new issues for those (#6283, #6284). Outline of moving pieces related to RequestContext, Base64 and Push changes: Dataset pg
Edit Files pg
Theme & Widget pg
Account pg
|
OK, tested it a little more - looks good, so moving into CR. |
Saw this warning in my server log today while on the create dataset form, and it reminded me to create this placeholder issue for the eventual move to PrimeFaces 7 (released Mar '19).
For a background on this component, see my comment on the PrimeFaces 6.2 Upgrade issue
Accessibility improvements are also a big selling point for the upgrade.
From the PrimeFaces upgrade guide:
https://github.com/primefaces/primefaces/wiki/Migration-Guide
6.2 to 7.0
Breaking Changes
Others
The text was updated successfully, but these errors were encountered: