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

Replace 'replace' for A2 compatiblity #265

Merged
merged 6 commits into from
May 11, 2016

Conversation

edgarmueller
Copy link
Contributor

@edgarmueller edgarmueller commented May 9, 2016

JSON Forms can't not be used with A2 migration mode, since we use replace in our code base.
I'm not very happy with this fix, since it introduces additional DOM elements, but I couldn't think of something else in order to keep the restriction that CSS layout styles should only be applied in case the respective layouts are visible.

Furthermore the PR also adds missing style capabilities for categorizations and groups.

This commit also contains two minor fixes that bugged me as well: that the person example is still called 'local' and that we did not yet include any usage info about JSON Forms in the README (although, of course, it's still WIP). If necessary I can also push them as single commits, I was just too lazy to do so :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 91.588% when pulling b7cdd40 on edgarmueller:master into 752e2f8 on eclipsesource:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 91.588% when pulling d1df77f on edgarmueller:master into 752e2f8 on eclipsesource:master.

</fieldset>
</uib-tab>
</uib-tabset>
<div class="jsf-categorization">
Copy link
Member

Choose a reason for hiding this comment

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

why not add the jsf-categorization class to the existing div?
i think, that the new div might destroy the grid that is created by bootstrap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Placing the class attribute onto the existing elements didn't work with the vertical/horizontal layout, that's why I also introduced the additional element here as well, but I didn't actually give it a try. Will do.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 91.588% when pulling 0ecbda2 on edgarmueller:master into 752e2f8 on eclipsesource:master.

@eneufeld
Copy link
Member

Looks good, thank you.

@eneufeld eneufeld merged commit a32ddae into eclipsesource:master May 11, 2016
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

3 participants