Skip to content

Conversation

@arunapa
Copy link
Contributor

@arunapa arunapa commented Oct 19, 2022

Screen Shot 2022-10-19 at 10 10 33 AM

Screen Shot 2022-10-19 at 10 11 37 AM

@arunapa arunapa linked an issue Oct 19, 2022 that may be closed by this pull request
@longshuicy
Copy link
Member

I got a 422 error when posting definition:
image

Comparing your post body with the examples:

  1. the context field should have another layer with the name of the definition as the key
  2. you are missing the "name" of the definition, e.g. DOI at the root level
{
   "metadataName":"doi",
   "metadataDescription":"doi",
   "context":"https://schema.org/doi",
   "fields":[
      {
         "name":"doi",
         "list":false,
         "widgetType":"TextField",
         "config":{
            "type":"str",
            "options":""
         },
         "required":true
      }
   ]
}

vs

{
    "name" : "DOI",
    "description" : "Digital object identifier",
    "context" : {
        "doi" : "https://schema.org/DigitalDocument"
    },
    "fields" : [
        {
            "name" : "doi",
            "list" : false,
            "widgetType": "TextField",
            "config": {
                "type" : "str"
            },
            "required" : true
        }
    ]
}

placeholder="Please enter field data type (e.g. int, str, enum)"
name="Field Data Type"
value={input.config.type}
onChange={(event) => { handleInputChange(index, "type", event.target.value); }}
Copy link
Member

@longshuicy longshuicy Oct 19, 2022

Choose a reason for hiding this comment

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

Since this right now is also an arbitrary list, could you also make it a dropdown?

"int": int,
"float": float,
"str": str,
"bool": bool,
"date": datetime.date,
"time": datetime.time,
"dict": dict,  # TODO: does this work?
"enum": str,  # TODO: need a way to validate enum,
"tuple": tuple,

Copy link
Member

Choose a reason for hiding this comment

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

For both the widget type and data type drop downs, can we use more layman's term? e.g. TextField --> Text Box; Select --> Dropdown, Int --> Integer, Float --> Number
You are welcome to improvise a little bit :D

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've added human readable names in both drop downs

image

image

@arunapa
Copy link
Contributor Author

arunapa commented Oct 27, 2022

Some more improvements I've added in the latest commit:

  1. Sanity checks while moving between stepper states (previously sanity was executed prior to invoking the API)

  2. Added a new "preview" text box so that the user can know what metadata definition is being sent to the backend
    image

  3. Clicking on the stepper can move between states now

@arunapa arunapa force-pushed the 86-metadata-definition-ui-latest branch from 2e4edf7 to 289b550 Compare October 27, 2022 14:29
@arunapa
Copy link
Contributor Author

arunapa commented Oct 27, 2022

Bugs addressed:

  1. Preview text should not show entries that have been deleted
  2. Duplicate menu button
  3. Fix the resize issue for the popup page

image

Pending bugs/improvements to be addressed in future PRs:
Making the navigation between stepper states more intuitive

</ListItemButton>
</ListItem>
</List>
<Divider/>
Copy link
Member

Choose a reason for hiding this comment

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

you have it duplicated here :-P
image

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've fixed this issue in the latest commit as well :)

Copy link
Member

@lmarini lmarini left a comment

Choose a reason for hiding this comment

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

Looks good. Just a few small improvements we can do on a separate branch and one potential bug.

  • We should probably use an example of the context using Dublin Core since it's pretty common
    {"abstract": "http://purl.org/dc/terms/abstract"}
  • The first section header is capitalized, but the second is not. We should be consistent, maybe change to "Create metadata definitions"
  • Maybe put to the two check boxes on the same line? "Contains list" "Required". What does "Contains list" do?
  • Can we make the metadata example being editable? So that they don't have to type everything, the curly brackets, etc?

I created an entry that includes a list but the list doesn't work when I try to create an instance of it. Just one field of type list. This is the definition in Mongo:

{ "_id": { "$oid": "635ff6063230ca72094d85d7" }, "name": "Abstract", "description": "Just another abstract", "context": { "abstract": "http://purl.org/dc/terms/abstract" }, "context_url": null, "fields": [ { "id": { "$oid": "635ff6063230ca72094d85d8" }, "name": "Abstract", "list": false, "widgetType": "Select", "config": { "id": { "$oid": "635ff6063230ca72094d85da" }, "type": "enum" }, "required": false } ], "creator": { "id": { "$oid": "632b39f29b06a45e416a33f4" }, "email": "[email protected]", "first_name": "Luigi", "last_name": "Marini" }}

@arunapa
Copy link
Contributor Author

arunapa commented Nov 1, 2022

I've addressed the following:

  1. Making the capitalization consistent
  2. Changing the default context example
  3. Making the context example editable for the user to modify quickly while submitting the form
  4. Checkboxes in same row

Screen Shot 2022-11-01 at 10 50 38 AM

Screen Shot 2022-11-01 at 10 50 44 AM

@lmarini lmarini merged commit 0e98928 into main Nov 2, 2022
@lmarini lmarini deleted the 86-metadata-definition-ui-latest branch November 2, 2022 19:15
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.

[Frontend] Implement UI for metadata definition API

4 participants