plantain-00 / schema-based-json-editor

A reactjs and vuejs component of schema based json editor.
MIT License
168 stars 38 forks source link

Time format not to spec #23

Closed tjwelde closed 5 years ago

tjwelde commented 5 years ago

Version(if relevant):

7.20.2

Environment(if relevant):

React

Code(if relevant):

schema:

{
  "$id": "Test",
  "$schema": "http://kilt-protocol.org/draft-01/ctype#",
  "properties": {
    "time": {
      "type": "string",
      "format": "time"
    },
  },
  "type": "object"
}

Expected:

According to https://json-schema.org/understanding-json-schema/reference/string.html#dates-and-times I expect the returned value from the editor to be: 20:20:39+00:00

Actual:

20:20

plantain-00 commented 5 years ago

The time-zone is optional for format: "time" I'm going to update the return value to be like 20:20:39

plantain-00 commented 5 years ago

After v7.21.0, add "step": 1, to return value to be like 20:20:39

{
      "type": "string",
      "format": "time",
      "step": 1
    }
tjwelde commented 5 years ago

Ah cool. Thanks for looking into it and fixing it. Why do we need "step": 1 ? I don't see it in the json-schema draft-07?

plantain-00 commented 5 years ago

step is a non-standard field, it's a UI related field, it comes from <input type="time">: https://developer.mozilla.org/zh-CN/docs/Web/HTML/Element/input/time

tjwelde commented 5 years ago

Ah, to add it to the input field. I think the step attribute should be 1 per default for time formats, because it is defined like that: https://tools.ietf.org/html/rfc3339#page-8 (search full-time). It is the definition used by draft 07 of json-schema. I wouldn't want to bloat my json-schema, so it works for this specific editor. A preparation step, to modify the schema to meet the needs of this editor is also not a nice solution. Could you make step="1" the default for time formats?

tjwelde commented 5 years ago

Withoutstep="1", the result is not verifiable against json-schema, so I would think it makes sense to have it as default

plantain-00 commented 5 years ago

It should be OK for v7.22.0

tjwelde commented 5 years ago

Man, you are quick. Works like a charm. Thanks a lot!