NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 28 forks source link

Allow the user to remove any non-required field from the EML #19

Closed laurenwalker closed 7 years ago

laurenwalker commented 7 years ago

There should be just a simple "X" icon next to each row or field, possibly only displayed on hover, that when clicked, removes that field from the EML.

Likewise, when a user clears a text input, that field should be removed from the EML.

This should likely be done in a two-step process:

amoeba commented 7 years ago

I'm about to finish up a first pass at a fully-working Taxa section so I'll work on this next.

amoeba commented 7 years ago

Alright, so I pushed my changes from earlier in the week after some testing and tweaking this morning. See https://github.com/NCEAS/metacatui/commit/a00142034ddff4a6ff89a202f719ea4c983b8be8.

Two things:

  1. Could I get a second look and some feedback on whether this should work well for the rest of the removable items?

    I implemented a generic method that should be applicable for all of the elements we want to make removable. For every removable item, a remove button is created that knows the CSS selector of the DOM parent it should remove from the DOM and the attribute name on the EML model it should change. It uses $().parents(...).first() to remove the appropriate DOM and uses _.without(...) to remove the appropriate model from the parent EML model.

  2. What should this look like?

laurenwalker commented 7 years ago

Thanks Bryce for getting this started. I think this is a really good start.

The method you describe works well for attributes that are set directly on the EML211 model with a model as the value. We'll have some other cases to check for:

I can "pretty up" the remove button today.

amoeba commented 7 years ago

Great overview, that's helpful. Regarding my (2), I'm happy to make the change unless you'd rather.

laurenwalker commented 7 years ago

I've styled the remove icon and made it hidden until the keyword set row is hovered over. When the remove icon is hovered over, the fields to be removed are outlined in a dashed red border.

Go ahead and keep adding the remove icon to the rows and I will clean them up as you go. There will need to be some section-specific styling and placement of the icon since each part of the editor is displayed differently.

whkgiiyaoo

amoeba commented 7 years ago

Great, thanks for doing that. I like what you came up with.

amoeba commented 7 years ago

The functionality for removing EMLText nodes is in and working but not applied everywhere it can. I'm about to wrap up support for removing child models too and that seems very doable. Will update today.

amoeba commented 7 years ago

Alright, I fixed a few bugs in funding serialization along with implementing removal of funding. I need to check my changes before committing. I'll do that this weekend. I'm using a shared pattern for these which currently supports models on the EML211 object and attributes of submodels (such as Project's funding attribute) so I see a nice way forward on this.

amoeba commented 7 years ago

Great progress on this today, including lots of bug squashing in other parts of the codebase. I have done some preliminary testing and things seem to be in order but I'm running into higher-level issues surrounding saving new package versions that prevent easy testing.

EML211View.handleRemove is supporting removing attributes on the EML, submodels on the EML model, and attributes from within submodels on the EML model.

On Monday I want to test things further and add in removal for People.

amoeba commented 7 years ago

Okay, I hooked in the remove button to the rest of the parties of Parties so that's all working as expected now.

Next thing is to do a scan of the various updateDOM functions to see about clearing out empty values before serialization.

amoeba commented 7 years ago

I came up with something pretty simple:

In each submodel's updateDOM method, before returning the objectDOM, use jQuery to filter out zero-length or whitespace-only nodes:

$(objectDOM).find("*").filter(function() { 
  return $.trim(this.innerHTML) === ""; 
} ).remove();

This may result in some performance issues with very large EML documents but it otherwise appears to work nicely.

I added this to:

which covers most everything.

laurenwalker commented 7 years ago

Will this keep nodes with   intact? That is how we create line-breaks in EMLText models.

amoeba commented 7 years ago

Yep!

amoeba commented 7 years ago

In:

<dataset>
  <title>test</title>
  <creator id="8236326755097470">
    <individualName>
      <givenName>&nbsp;</givenName>
      <surName>Mecum</surName>
    </individualName>
    <organizationName> </organizationName>
    <positionName> </positionName>
  </creator>
  <contact id="3417637806768093">
    <individualName>
      <givenName>Bryce</givenName>
      <surName>Mecum</surName>
    </individualName>
    <electronicMailAddress>m</electronicMailAddress>
    <userId directory="https://orcid.org">http://orcid.org/0000-0002-0381-3766</userId>
  </contact>
</dataset>

Out:

<title>test</title>
<creator id="8236326755097470">
    <individualname>
        <givenname>&nbsp;</givenname>
        <surname>Mecum</surname>
    </individualname>
</creator>
<contact id="3417637806768093">
    <individualname>
        <givenname>Bryce</givenname>
        <surname>Mecum</surname>
    </individualname>
    <electronicmailaddress>m</electronicmailaddress>
    <userid directory="https://orcid.org">http://orcid.org/0000-0002-0381-3766</userid>
</contact>
amoeba commented 7 years ago

Pen here to play with http://codepen.io/petridish/pen/wdBYJV

laurenwalker commented 7 years ago

Sections missing remove buttons:

Also some remove buttons are not working. I'll create specific tickets for those bugs.