6919 preview tools #7311
6919 preview tools #7311
Conversation
…eferences in guides to explore tools [ref #6919]
…on btn to Style Guide [ref #6919]
The rule is "The eyeball is present if the Preview tab is present." There are edge cases to consider around guestbook, terms of use, restricted data, etc. For now we at least check if the appropriate tools are in place.
Our goal is to include file-download-popup-fragment.xhtml onto another location, the Preview tab on the file page, while still displaying it as a popup on that page. In the Preview tab the input fields weren't showing up and we discovered that commenting out "binding" made them appear. Commenting out "binding" resulted in validation not working. Additionally removing process=@this along with much more refactoring, got validation working. Bean validation was added as well. The hope is that with this commit in place, we can switch back to trying to get the extra "include" (mentioned above) working. Some things to note about this commit: - Custom UIInput fields have been removed in favor of standard Bean Validation. - The GuestbookResponse field was removed from FileDownloadHelper because it was confusing to sometimes set it and other times pass it in as argument to various methods. Now we always pass it in. - Various unused code was deleted. - process=@this wasn't removed from the "package" code and it's unknown if that code is working. Conflicts: src/main/java/ValidationMessages.properties
This commit does two things: - Include file-download-popup-fragment.xhtml (where guestbook and terms live) on the Preview tab. - Allow files to be previewed in more scenarios, such as when datasets have a guestbook or terms. There are a couple TODO/FIXME items not addressed: - <p:focus context="downloadPopup"/> was commented out. - @Form is used in a place where it could be more precise.
…ic to btn txt on file pg [ref #6919]
Without this update, the failure case was this: 1. When there are required fields, click Accept without filling them in. 2. Fill in all required fields and click Accept. 3. The form remains and you cannot see the preview.
Conflicts: src/main/java/ValidationMessages.properties
Lots of edits were made previously in this branch.
|
In 767882b I implemented multiple types and removed hasPreviewMode. I'm sending this back into code review. |
| update="guestbookUIFragment"> | ||
| <f:param name="DO_GB_VALIDATION" value="true"/> | ||
| <p:commandButton styleClass="btn btn-default" value="#{bundle.acceptTerms}" rendered="#{guestbookResponse.fileFormat == 'externalTool'}" | ||
| action="#{fileDownloadHelper.writeGuestbookAndLaunchExploreTool(guestbookResponse, fileMetadata, tool)}" |
There was a problem hiding this comment.
If we are rendering the buttons based on the gbResponse file formats, can we just have one method for Launching the explore tool, world map and package popup that decides what to do based on the file format. That could reduce three buttons to one.
There was a problem hiding this comment.
A related thought: @djbrooke has given the ok to start removing world map related buttons and such, so if that helps simplify the code... (I've started to do that in the kebab issue / edit buttons)
There was a problem hiding this comment.
Yes - I have on my list to start clearing out Worldmap issues from Github and to create an issue to pull out any Worldmap/Geoconnect code specific in Dataverse, so it's good to get a headstart.
There was a problem hiding this comment.
I just looked at this and I don't see a lot of value in refactoring. The complexity has to live somewhere and right now a decision is made on the front end. I think it's fine as it is.
We've switched to Bean Validation, which is done centrally instead of being only in the UI.
|
I addressed code review comments in e73d2f4 and I'm moving this back to code review. |
|
@pdurbin I accidentally merged, then reverted this pr. In the process of attempting to deploy, I'd found that it appears to conflict with the mail regex branch on flyway script names. Mail regex is ready to be merged and preview still needs to be tested so was hoping you could fix the script name here. I don't think we can reopen this pr but the preview branch still exists. Can you fix there and make a new pr? |
What this PR does / why we need it:
This pull request does two things:
Prior to this pull request, the Preview tab was only available for files that are publicly downloadable.
The way to create a "preview only" tool is to use the new "preview" type.
Note that explore tools can continue to appear under the Preview tab (on the file page) if they have hasPreviewMode set to true, as before.This was removed in 767882bWhich issue(s) this PR closes:
Closes #6919 (Preview Tools - New external tool preview type config setting, icon btn on dataset/file pgs)
Some commits have been added to address #4429 (Revise Explore Tools documentation to reflect modularity )
Special notes for your reviewer:
Stuff to notice:
Some questions I have:
Suggestions on how to test this:
The code having to do with the terms/guestbook popup was extensively reworked so it would be good to test...
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Screenshot 1: preview only example
Below is an example of a "preview only" tool. Note that the button in the Preview tab says "Open in New Window" and the tool is not present under the "Access File" dropdown.
Screenshot 2: Terms + Guestbook
Below is an example of terms and a guestbook being presented in a Preview tab. The tool happens to be an explore tool and shows up under "Explore Options" under the "Access File" dropdown.
Is there a release notes update needed for this change?:
Yes, a release note has been included.
Additional documentation:
Doc changes included.