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

fixes #1611 #1612

Merged
merged 4 commits into from
May 2, 2017
Merged

fixes #1611 #1612

merged 4 commits into from
May 2, 2017

Conversation

danielstorey
Copy link
Member

_supportedLayout was not working at all meaning left, right and full were always available even if a plugin's properties.schema was limited to half-width or full-width only. This PR addresses the issue in two areas:

Sidebar component list - when a component is selected only the options specified in the properties schema are available.
Page editor - if a component is full-width only then the component-move arrow icons will not be rendered. If half width only then the full width arrow will not be rendered

var supportedLayout = properties.hasOwnProperty('._supportedLayout').enum;
if (properties && properties.hasOwnProperty('_supportedLayout')) {
var supportedLayout = properties._supportedLayout.enum;
var availablePositions = {
Copy link
Member

Choose a reason for hiding this comment

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

@danielstorey I think availablePositions is also used to find positions that are not already occupied as set in setupFilters function. Assigning a new value to availablePositions here allows the user to insert two left (or right) positioned components.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok good point...so just need to combine both checks

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Latest commit should do it!

Copy link
Member

@brian-learningpool brian-learningpool left a comment

Choose a reason for hiding this comment

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

Nice job, @danielstorey! This has been an issue for some time.

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.

5 participants