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

Validation errors interface (make long codelists collapsible) #22

Open
duncandewhurst opened this issue Nov 4, 2022 · 15 comments
Open
Labels
interface quick Issues that shouldn't take long to resolve

Comments

@duncandewhurst
Copy link
Contributor

The current validation errors interface has a lot of repetition for errors of the same type:

image

I think that it would be good to group errors by type, in a similar way to the additional checks are grouped (see #13):

image

My thinking is that for errors with fewer than 3 occurrences, the errors table should be expanded by default. Otherwise, it should be collapsed by default.

Fonts, font sizes and colours etc. are all placeholders. I think there is probably a more elegant way of doing the expanding/collapsing of the error tables too.

@duncandewhurst
Copy link
Contributor Author

536a951 renames the panel and adds the introductory sentence.

@duncandewhurst
Copy link
Contributor Author

See #4 (comment) for a spreadsheet with a list of error messages/types to group by.

@odscjames
Copy link
Collaborator

Note from #32 (comment) :

Add quotes to string values, for example, in the image below, 'yes' should be enclosed in double quotes because it is a string value. Otherwise, it would be impossible to distinguish between true and "true".

@odscjames
Copy link
Collaborator

Showing the value on missing required fields isn't actually that helpful.

missing-req-fields

Should we show what field in missing instead?

@odscjames
Copy link
Collaborator

How should we show long codelists?

long-codelists

odscjames added a commit that referenced this issue Nov 17, 2022
@duncandewhurst
Copy link
Contributor Author

Showing the value on missing required fields isn't actually that helpful.

...

Should we show what field in missing instead?

Can we leave value blank (since a missing field can have no value) and append the missing field to path? e.g. networks/0/organisations/0/id - that seems logical to me.

How should we show long codelists?

We could either have them collapsed to begin with and allow the user to expand them, or we could replace the list of values with a link to codelist's entry in the reference documentation, e.g. country. I don't have a particular preference either way, happy for you to go with whatever is easiest to implement and maintain.

@duncandewhurst
Copy link
Contributor Author

Is making the error lists collapsible still on the to-do list? Whilst testing with the data from Brazil, I sometimes had upward of 1,000 errors which meant a lot of scrolling!

@odscjames
Copy link
Collaborator

odscjames commented Nov 29, 2022

Todo list:

  • make long codelists collapsible
  • missing required fields - remove value column entirely and add missing field name to end of path
  • show/hide button on each section needs work - fewer than 3 occurrences, the errors table should be expanded by default. Otherwise, it should be collapsed by default. Probably different labels on the button for each state?

Is this all for this issue?

@duncandewhurst
Copy link
Contributor Author

duncandewhurst commented Nov 30, 2022

  • show/hide button on each section needs work - fewer than 3 occurrences, the errors table should be expanded by default. Otherwise, it should be collapsed by default. Probably different labels on the button for each state?

Your suggestion sounds good for the defaults. It would be great to get some input from @codemacabre on a good UI for expanding/collapsing as the buttons feel a bit clunky and I don't know how to improve it.

From a quick check, I see two more issues outstanding:

  • Fix issues with the Show/Hide buttons controlling the wrong elements, e.g. in these results, clicking the Show/Hide button for 'Value does not match pattern' actually shows/hides the errors for 'Field name does not match pattern'.
  • Apply code formatting to text in backticks, e.g.

image

@odscjames
Copy link
Collaborator

Ticking off backticks comment, as that already has it's own issue: #35

@odscjames
Copy link
Collaborator

Ticking off show/hide comment, as that is moved to #65

@odscjames
Copy link
Collaborator

missing required fields - remove value column entirely and add missing field name to end of path

Just realised - this is fine when we only show JSON paths. When we start showing spreadsheet cells or GeoJSON paths (#56 & #57 ) we'll have to come back to this anyway. For spreadsheet cells we can't add to end of path and for GeoJSON that might get complicated.

Can we stick to having the value column, which should show the id of missing field? And the path column can show the location information, just like all the other tables?

@duncandewhurst
Copy link
Contributor Author

Can we stick to having the value column, which should show the id of missing field? And the path column can show the location information, just like all the other tables?

I think that's fine, but let's rename the value column to 'missing field'.

odscjames added a commit that referenced this issue Dec 2, 2022
Show/Hide links, correct duplicate CSS id
Check right variables

#22
@odscjames
Copy link
Collaborator

Fix issues with the Show/Hide buttons controlling the wrong elements
but let's rename the value column to 'missing field'.

Ready for testing on live

@duncandewhurst
Copy link
Contributor Author

duncandewhurst commented Dec 7, 2022

Changes look good. Just one outstanding task in #22 (comment). Edit: I've renamed the issue to reflect the outstanding task.

@duncandewhurst duncandewhurst changed the title Validation errors interface Validation errors interface (make long codelists collapsible) Dec 7, 2022
@duncandewhurst duncandewhurst added the quick Issues that shouldn't take long to resolve label Dec 7, 2022
odscjames added a commit that referenced this issue Dec 15, 2022
…ion: Print JSON values nicer

* quote strings
* print lists as lists, with longer ones collapsed by default

#20
#13
#22
odscjames added a commit that referenced this issue Dec 20, 2022
…ion: Print JSON values nicer

* quote strings
* print lists as lists, with longer ones collapsed by default

#20
#13
#22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interface quick Issues that shouldn't take long to resolve
Projects
None yet
Development

No branches or pull requests

2 participants