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

6339 subjects disappear on edit #6356

Merged
merged 24 commits into from
Dec 3, 2019
Merged

Conversation

sekmiller
Copy link
Contributor

New Contributors

Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Note that we use the "closes" syntax below to trigger Github's automation to close the corresponding issue once the pull request is merged.

Thanks for your contribution to Dataverse!

Related Issues

Pull Request Checklist

  • Unit [tests][x] NA
  • Integration [tests][x]: NA
  • Deployment requirements: [x] None
  • Documentation NA
  • Merged latest from "develop" [branch][x] and resolved conflicts

@coveralls
Copy link

coveralls commented Nov 8, 2019

Coverage Status

Coverage decreased (-0.0003%) to 19.562% when pulling 51881e7 on 6339-subjects-disappear-on-edit into 884d73f on develop.

@kcondon kcondon self-assigned this Nov 12, 2019
@kcondon
Copy link
Contributor

kcondon commented Nov 12, 2019

@sekmiller This still happens with Chrome:

  1. for both subject and languages, and likely other controls of the same list-type
  2. to reproduce, scroll past the initially visible items, click on the name of the item, not the checkbox, and the entire list is hidden, though saving saves any selected items.

@kcondon kcondon assigned sekmiller and unassigned kcondon Nov 12, 2019
@mheppler mheppler assigned mheppler and unassigned mheppler Nov 18, 2019
@sekmiller sekmiller self-assigned this Nov 20, 2019
@mheppler
Copy link
Contributor

mheppler commented Nov 21, 2019

Reviewed latest changes to the multiple attribute of the selectCheckboxMenu component, and seeing inconsistent behavior between the create and edit form. First, you will see that the edit version of the "Select..." dropdown has a gray background color, while the create version is white. Second, selecting checkboxes displays the values in the dropdown for the edit version, while it is broken in the create version. (See screenshots.)

So there appears to be something going on with the EDIT vs CREATE versions of this form component.

EDIT
Screen Shot 2019-11-21 at 1 50 29 PM

Screen Shot 2019-11-21 at 2 11 50 PM

CREATE
Screen Shot 2019-11-21 at 1 50 51 PM

Screen Shot 2019-11-21 at 2 11 02 PM

Also, on the metadataFragment, there is another version of the old scrolling selectManyCheckbox component for compound "subdsf" fields that will need to be swapped out to the new selectCheckboxMenu component. There also an old selectManyCheckbox component on the advanced search pg for subjects and other dynamic fields, that will need to be updated.

@mheppler
Copy link
Contributor

Latest changes moved the selected values of the selectCheckboxMenu component to custom plain text outside the component, rather than displaying in the component as the component functions in the PrimeFaces showcase, out of the box.

Screen Shot 2019-11-22 at 7 52 58 AM

Hopefully it is possible to solve the inconsistency of this component between the edit and create workflows, by fixing the component's intended functionality, and not a custom code workaround.

@pdurbin
Copy link
Member

pdurbin commented Nov 22, 2019

One question is have about the new component is if it allows authors to put the subjects in order. Can they make is so that "Physics" appears before "Chemistry"? The idea here is that the dataset is a little more about Physics than Chemistry.

@sekmiller
Copy link
Contributor Author

@pdurbin Nope. Alphabetical order. Is it worth trying to add ordering as an enhancement?

@pdurbin
Copy link
Member

pdurbin commented Nov 22, 2019

@sekmiller nah. I'd sooner working on adding many, many more subjects before we prioritize the concept of "primary subject". Thanks.

@mheppler
Copy link
Contributor

mheppler commented Dec 2, 2019

Here is the current broken UX as of the last commit to revert the AJAX changes from the component.

CREATE WORKFLOW

  • Check boxes, selections not displayed in dropdown (broken compared to component showcase)

EDIT WORKFLOW

  • Selected values displayed in dropdown
  • If nothing selected, check boxes, selections not displayed in dropdown (same as create)
  • If values already selected, check more boxes, selections displayed in dropdown (THIS is how it behaves in the showcase, and how it should behave across all workflows)
  • Unselect everything and input if left blank, without "Select..." label text returning (broken compared to component showcase)

(forgoing “Select…” when empty.)
@sekmiller
Copy link
Contributor Author

sekmiller commented Dec 2, 2019

With Item(s) selected:

Screen Shot 2019-12-02 at 2 38 53 PM

While Editing:

Screen Shot 2019-12-02 at 2 38 29 PM
On create or Edit with nothing selected:

Screen Shot 2019-12-02 at 2 37 42 PM

@djbrooke
Copy link
Contributor

djbrooke commented Dec 3, 2019

We discussed in standup and I made the decision to move this as is. It is not perfect but it addresses a production bug.

@pdurbin
Copy link
Member

pdurbin commented Dec 3, 2019

I would say the user experience for picking a language from a long list is much improved. Now there's search! 😄

@kcondon kcondon self-assigned this Dec 3, 2019
@kcondon kcondon merged commit 29940f0 into develop Dec 3, 2019
@kcondon kcondon deleted the 6339-subjects-disappear-on-edit branch December 3, 2019 18:51
@djbrooke djbrooke added this to the 4.19 milestone Dec 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Subjects disappear when clicked in metadata editing
6 participants