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

Global metadata on demand #483

Merged
merged 20 commits into from
Nov 1, 2023
Merged

Global metadata on demand #483

merged 20 commits into from
Nov 1, 2023

Conversation

garrettmflynn
Copy link
Member

@garrettmflynn garrettmflynn commented Oct 30, 2023

fix #424

This PR allows users to pull up global metadata on demand and edit those properties. The Subject, Source Data, and File Metadata pages all have a new button near the header to toggle Global Metadata editing.

Additional related fixes were included, such as:

  • Maintain form state on updates (i.e. accordions will stay open when updated with new globals)
  • Do not allow users to specify an empty value if there are globals. Auto-fill with global when cleared—as this is how the property will be interpreted (and rendered on page reload)
  • Ensure proper deletion of properties that are set to an empty string / undefined (as previously these would not clear...)

User Testing Issues

  • Made all single-session fixes to experimenter name but did not change the global level
  • Missed the idea of the earlier global metadata pages (did not read the description, or was not clear?)
  • would be nice to be able to specify the interface kwargs (Source Data Page) at a global / subject-specific level.
  • Did not fill out global subject metadata on the Subject Table however, such as species and age [commented on how this was annoying to specify for each session]
  • unclear how to triage inspector report feedback (e.g. global metadata is usually the best way to go for recurring issues)

@garrettmflynn garrettmflynn self-assigned this Oct 30, 2023
@CodyCBakerPhD
Copy link
Collaborator

@garrettmflynn small conflict here

@garrettmflynn
Copy link
Member Author

Alright this is properly merged now

@CodyCBakerPhD
Copy link
Collaborator

This design wasn't very intuitive for me personally

I went to the file metadata page and started to enter a couple manual values for a field that should have been globally defined, then I clicked the new button and this is what it said

image

Next I manually navigated back to the global metadata page for NWB file metadata, added the field there, hit 'save and continue', and went back to the session level pages, which now have the global value

But when I still click on the new 'Edit Global Metadata' button I see the same screen with no values to update, and pressing update didn't seem to change anything on any session pages (even after I manually altered some of them to new values)

Maybe this is due to a bug, but can you describe what the exact workflow for the usage of this button is supposed to be? We had discussed it simply being a link back to the global metadata page (to raise user awareness of the early step that they only realize is useful later on in the process)

@garrettmflynn
Copy link
Member Author

There were a few bugs—so I've fixed those now :)

Generally, though, I see where the confusion lies. What I was describing was editing the global metadata data structure through a button that opens all / specific properties on demand. As such, the workflow is to (1) press the button to open all the global metadata on that page, (2) update any properties you'd like, (3) press the Update button, (4) re-render the form with the new globals filled in. I've went through this many times for the latest fixes—so it should just work now.

We'd also described a pop-up when entering the global metadata pages so that users were forced to explicitly decide to add this metadata or not early in the pipeline—though I have not added this because of the availability of the global data through these buttons.

@CodyCBakerPhD
Copy link
Collaborator

Oh, I see - I'll give it another fresh try then

Some conflicts now though

@CodyCBakerPhD
Copy link
Collaborator

Also looks like tests are failing after the resolution

@garrettmflynn
Copy link
Member Author

Got it my bad. Should be fixed now

@CodyCBakerPhD
Copy link
Collaborator

Ahh, yes... This is much better when it's working lol

Looks great!

@CodyCBakerPhD
Copy link
Collaborator

The only thing I'd change, which I leave as a simple follow-up, would be to remove the button from the 'source data' page since it has no relevance to that specific step

Attentive users will also have their attention drawn to the new button when it then appears on the metadata page right after thatt too

@garrettmflynn
Copy link
Member Author

One of the drivers of this PR in the first place was Felix's suggestion to include a way to specify global kwargs for the interfaces—so wouldn't we want to keep this for Source Data?

@CodyCBakerPhD
Copy link
Collaborator

Ohhh I did not notice it was global source data on that page

image

In that case it's just the name of the button that needs to change, it still says 'metadata'

image

Also might as well skip any/all interface banners that don't have extra arguments (that result in that 'no additional properties to render' text)

@CodyCBakerPhD CodyCBakerPhD merged commit da7b93f into main Nov 1, 2023
7 checks passed
@CodyCBakerPhD CodyCBakerPhD deleted the global-metadata-on-demand branch November 1, 2023 15:00
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.

[Feature] Add 'Set Global Metadata' page link at top of session-level file metadata page
2 participants