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

Use schema title attribute where appropriate #1335

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

sdirix
Copy link
Member

@sdirix sdirix commented Apr 18, 2019

The label utility will now return the schema title if no other label was
defined via the ui schema.

Within the ui schema a label can be optionally defined for each control.
Previously if there was none, we fell back to auto-generating a label
from the given control scope. Now we will check the schema title
beforehand and return that if no label was defined for the control.

This also affects the ui schema generation which will no longer
auto-generate labels based on each control's scope.

Includes testcases.

Implements #1240

@sdirix sdirix force-pushed the use-titles branch 3 times, most recently from 588120a to cbda110 Compare April 18, 2019 10:24
@sdirix sdirix marked this pull request as ready for review April 18, 2019 10:26
@sdirix sdirix requested a review from eneufeld April 18, 2019 10:41
Copy link
Member

@eneufeld eneufeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, than you. Just a small question.

if (schemaElement && typeof schemaElement.title === 'string') {
return schemaElement.title;
}
if (typeof controlElement.scope === 'string') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this check?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did it because the check was already in the previous code as controlElement.scope !== undefined. I can remove it if you want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, its fine. I missed the check that was there.

The label utility will now return the schema title if no other label was
defined via the ui schema.

Within the ui schema a label can be optionally defined for each control.
Previously if there was none, we fell back to auto-generating a label
from the given control scope. Now we will check the schema title
beforehand and return that if no label was defined for the control.

This also affects the ui schema generation which will no longer
auto-generate labels based on each control's scope.

Includes testcases.
@sdirix
Copy link
Member Author

sdirix commented Apr 18, 2019

Rebased on master for a working Travis build

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.5%) to 85.627% when pulling 025ce39 on sdirix:use-titles into 083594e on eclipsesource:master.

@edgarmueller edgarmueller merged commit 94bec5b into eclipsesource:master Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants