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

3175 edit dvpage bug #7628

Merged
merged 6 commits into from Feb 24, 2021
Merged

3175 edit dvpage bug #7628

merged 6 commits into from Feb 24, 2021

Conversation

sekmiller
Copy link
Contributor

What this PR does / why we need it:
On create DV if the user went into custom metadata blocks before filling in the required "alias" and "category" fields the "Done" button to close the metadata block view wouldn't work. (also there was no indication why 'hey dummy, put in an alias for your new dv')
Also fixed an older bug where you could edit the input levels of an optional mdb and then deselect it.

Which issue(s) this PR closes:

Closes #3175
Closes #1198

Special notes for your reviewer:
@mheppler I used "jsf:disabled" for the 1198 fix. It seems to work but it's not used anywhere else. Kosher?

Suggestions on how to test this:

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

Is there a release notes update needed for this change?:

Additional documentation:

Copy link
Contributor

@scolapasta scolapasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I selected "request changes" instead of "comment" because I think the line I commented on will need to change; I could be wrong...

<p:selectOneRadio id="OptionsRadio#{dsft.id}" value="#{dsft.requiredDV}" widgetVar="OptButton#{dsft.id}" disabled ="#{DataversePage.editInputLevel == false}"
rendered="#{!dsft.hasChildren and !((!dsft.hasChildren and dsft.required) or (dsft.hasChildren and dsft.hasRequiredChildren))}">
<p:selectOneRadio id="OptionsRadio#{dsft.id}" value="#{dsft.requiredDV}" widgetVar="OptButton#{dsft.id}" disabled ="#{DataversePage.editInputLevel == false}"
onclick="#{DataversePage.toggleInputLevel(mdb.id, dsft.id)}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this onclick doing? onclick is for calling javascript, but what you have here is a backing bean (java) call, so I don't think this would be called each time you click?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it in because in my pre-testing I thought I had seen a case where changes to required/optional weren't being saved and because the method in the backing bean wasn't being used anywhere. I thought they were related so I put in the onclick. After removing both it seems to work so I'll chalk it up to bad pre-testing.

@@ -287,6 +287,7 @@
<p:ajax update="@widgetVar(optionBlock)" />
</input>
<input type="checkbox" jsf:itemValue="#{mdb}" id="#{mdb.idString}" jsf:value="#{mdb.selected}"
jsf:disabled="#{DataversePage.editInputLevel == true}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this still work without the jsf: passthrough? I would imagine it would. The other sibling basic HTML input's above it do not have it on their disabled attributes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work without the jsf: because if the disabled tag is there even if it contains "false" it disables the component. The others on lines 284 and 286 have disabled="disabled" and the real decision work is done on the jsf:rendered.

@@ -511,18 +511,6 @@ public void hideDatasetFieldTypes(Long mdbId) {
}
setEditInputLevel(false);
}

public void toggleInputLevel( Long mdbId, long dsftId){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another question: this is being removed, but I don't see a corresponding removal if it being called? Was it never used (or am I just missing it)? (is it related to what was in the onclick that got removed, i.e. was that supposed to call this and not something in the backing bean?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was what was being called in the (now removed) onclick on the option buttons and nowhere else.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Review 🦁 to QA 🔎✅ Feb 22, 2021
@scolapasta scolapasta removed their assignment Feb 22, 2021
@kcondon kcondon assigned kcondon and unassigned kcondon Feb 24, 2021
@kcondon kcondon merged commit f642c02 into develop Feb 24, 2021
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Feb 24, 2021
@kcondon kcondon deleted the 3175-edit-dvpage-bug branch February 24, 2021 20:13
@djbrooke djbrooke added this to the 5.4 milestone Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
5 participants