allow Material-ui boolean checkboxes to have descriptions

[cfluke-twilio]

Reasons for making this change

Descriptions for boolean checkboxes were being ignored.

If this is related to existing tickets, include links to them as well. Use the syntax fixes #[issue number] (ex: fixes #123).
fixes #2203

Checklist

• I'm updating documentation
  • I've checked the rendering of the Markdown text I've added
• I'm adding or updating code
  • I've added and/or updated tests
  • I've updated docs if needed
• I'm adding a new feature
  • I've updated the playground with an example use of the feature

Deploy preview

Once CI finishes, you should be able to view a deploy preview of the playground from this PR at this link: https://deploy-preview-[PR_NUMBER]--rjsf.netlify.app/

[cfluke-twilio]

Is the correct fix to this issue, to add the schema.description check in CheckboxWidget?
to make it similar to core?

this appears to be a problem in material-ui, antd, and sematic-ui as well.

The current solution does currently make it double appear in default theme. This seems to be caused by the WrapIfAdditional and DefaultTemplate adding the description as well.

[epicfaace]

Are you sure this is the right approach? This same function appears in utils.js --

react-jsonschema-form/packages/core/src/utils.js

Lines 389 to 407 in 1ae4f39

	export function getDisplayLabel(schema, uiSchema, rootSchema) {
	const uiOptions = getUiOptions(uiSchema);
	let { label: displayLabel = true } = uiOptions;
	if (schema.type === "array") {
	displayLabel =
	isMultiSelect(schema, rootSchema) ||
	isFilesArray(schema, uiSchema, rootSchema);
	}
	if (schema.type === "object") {
	displayLabel = false;
	}
	if (schema.type === "boolean" && !uiSchema["ui:widget"]) {
	displayLabel = false;
	}
	if (uiSchema["ui:field"]) {
	displayLabel = false;
	}
	return displayLabel;
	}

-- is there another way to fix this?

[cfluke-twilio]

Hi @epicfaace, see my comment above.
The easiest, simplest fix for material ui is to add:
{schema.description && ( <DescriptionField description={schema.description} /> )}

into the CheckBox widget itself. but the problem is in 2 other theme libraries as well. So I was looking at a more global solution to keep the pattern consistent across the various themes.

From a library design perspective, do you want widgets to be responsible for their own descriptions? or allow WrapIfAdditional pattern?

Should I move ahead with only the material-ui fix to move this forward?

[epicfaace]

From a library design perspective, do you want widgets to be responsible for their own descriptions? or allow WrapIfAdditional pattern?

Yeah, let's move ahead with the material-ui fix to move this forward for now. Unfortunately, as you note, there is an inconsistency between themes in which some themes have widgets responsible for their own descriptions, while others don't. Maybe we can consider improving that to make it consistent in a future major version release.

[epicfaace]

Thanks, can you add a snapshot test for this as well?

[epicfaace]

Looks good -- thanks!

[epicfaace]

From a library design perspective, do you want widgets to be responsible for their own descriptions? or allow WrapIfAdditional pattern?

Sorry, @cfluke-twilio I'm a bit confused. Why do descriptions work for the TextWidget in material-ui, even though there's no code like the one in this PR for TextWidget?

[cfluke-twilio]

TextWidget is calling the getDisplayLabel from the core utils here
The FieldTemplate uses that getDisplayLabel to determine if it should add a Typography and rawDescription here

The core utils has getDisplayLabel return false for checkboxes.

[Morriz]

bump

[Morriz]

We have created a lot of hacky workarounds to overcome all of these weird exceptions. Maybe we should consider adding to your core, as we depend on it sooooo much @epicfaace. Thanks for all the hard work. I do get the feeling that the maintainers are all paid volunteers, and it would be awesome if this project could get adopted by a major corp to start funding development.