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

Add required star when table format #762

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jneiluj
Copy link
Contributor

@jneiluj jneiluj commented May 24, 2020

Q A
Is bugfix?
New feature? ✔️
Breaks backward-compatibility?
Tests pass? ✔️
Fixed issues
Updated README/docs?
Added CHANGELOG entry?

Add a red star to cell header in table format for Spectre theme.

@jneiluj jneiluj changed the title Add required star in when table format Add required star when table format May 24, 2020
@schmunk42
Copy link
Collaborator

Could you add a test/use-case/example, I was unable to verify.

@jneiluj
Copy link
Contributor Author

jneiluj commented May 25, 2020

Could you add a test/use-case/example, I was unable to verify.

{
  "title": "Table with red star",
  "type": "array",
  "format": "table",
  "items": {
    "type": "object",
    "required": [
      "string1",
      "string2"
    ],
    "properties": {
      "string1": {
        "type": "string"
      },
      "string2": {
        "type": "string"
      },
      "string3": {
        "type": "string"
      }
    }
  }
}

Screenshot 2020-05-25 at 16 59 08

Tell me if more is needed.

schmunk42 added a commit that referenced this pull request May 26, 2020
@schmunk42
Copy link
Collaborator

schmunk42 commented May 26, 2020

Could you add the required class for (all) the other themes also.
I created a test here: fe99231 (currently checks spectre and bootstrap4)

Branch: https://github.com/json-editor/json-editor/compare/jneiluj-enhance-spectre-theme

I think I can not push onto your PR branch.

@germanbisurgi
Copy link
Collaborator

status?

@jneiluj
Copy link
Contributor Author

jneiluj commented Jun 3, 2020

I will do, I need time (quite busy atm).

@afmartin3z
Copy link

status?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants