Fix: Title is required to set notify popup dialog label (fixes #294)

[kirsty-hames]

Fixes #294

Fix

• Set title as required. Without a title, the popup dialog won't be properly identified by assistive technologies.

[guywillis]

👍

[cahirodoherty-learningpool]

👀

[swashbuck]

@kirsty-hames Does this need fixed in component.schema.json as well?

[kirsty-hames]

@kirsty-hames Does this need fixed in component.schema.json as well?

Thanks @swashbuck. I wasn't too sure about this as there's a lot of inconsistencies between required properties across component.schema.json and properties.schema. For example, there are 18 properties set to "required": true in properties.schema, but only _graphic.src is set as required in component.schema.json..

Is this just an oversight? If so I'll raise a separate issue and just set title as required in this PR.

[swashbuck]

@joe-allen-89 @taylortom Any thoughts on Kirsty's question above? What are the guidelines for using "required" true vs. false?

[joe-allen-89]

@kirsty-hames @taylortom @swashbuck This looks to be an oversight, all the schemas were set up using adapt-octopus which seems to factor in whether the attribute has a default value for some reason before including it in the new schema - https://github.com/adapt-security/adapt-octopus/blob/d7c31730567ecbc01cbbb9ee6caa5ae534d95df7/lib/SchemaNode.js#L184

I'm just looking at a few other schemas to double check but this is likely to be an issue across all repos as they were all set up in the same way.

[taylortom]

This one isn't an oversight actually, it's as designed.

In our case, defining an attribute as required while also having a default value is unnecessary, as the value will be set to the default (although the JSON schema allows anything to be used as a default value, even something that doesn't itself pass the schema valuation - but there's no reason for us to do this, or indeed expect it).

Therefore for brevity, we decided to remove the required definition if there's a default present. That being said, there's no problem with re-adding required definitions if there's a good reason though.

---

It's also worth noting that the required notation has changed with the new JSON schemas, see:
https://json-schema.org/learn/getting-started-step-by-step#define-required-properties

Summary

No longer a boolean, now an array:

{
  "$schema": "https://json-schema.org/draft/2020-12/schema",
  "$id": "https://example.com/product.schema.json",
  "title": "Product",
  "description": "A product from Acme's catalog",
  "type": "object",
  "properties": {
    "productId": {
      "description": "The unique identifier for a product",
      "type": "integer"
    },
    "productName": {
      "description": "Name of the product",
      "type": "string"
    },
    "price": {
      "description": "The price of the product",
      "type": "number",
      "exclusiveMinimum": 0
    }
  },
  "required": [ "productId", "productName", "price" ]
}

[joe-allen-89]

@taylortom I'd misunderstood that by setting a default forms expects a value, thinking the default could be changed to blank and potentially causing an issue.

As there's no defualt value for title, we'll either need to add a default value or add it as required.

[taylortom]

@joe-allen-89 Ah yeah I think that's how legacy works actually, as there's no server validation.

With v1, you'll get the default from both the UI (when the form's rendered) and the API server (when the data is saved), so if you remove the default value that's been populated for you in the form, it should be re-added again by the server API when the validation runs.

The required property then sits on top of everything else and returns an error if no value has been provided for that field.

[kirsty-hames]

This PR will need testing to confirm the latest schema commit a5122c8. @joe-allen-89 / @taylortom, are you able to confirm please?

[github-actions]

🎉 This PR is included in version 6.11.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀