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

Introduced a new "design" global option #209

Closed
wants to merge 3 commits into from

Conversation

javiereguiluz
Copy link
Collaborator

This is the first issue related to #208

by the new "design -> assets -> css" and "design -> assets -> js" options
The deprecation should be transparent and all applications should keep
working as previously.

In addition, this commits splits the DI extension class into smaller
and easier to manage sections.
->then(function ($v) {
// if the new option is defined, don't override it with the legacy option
if (!isset($v['design']['assets']['js'])) {
$v['design']['assets']['js'] = $v['assets']['js'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this (and other) instruction throw an error if the $v['design'] is not set yet ? 😕

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've changed it here: 982f401 Let me know if it's better that way. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great 👍

@Pierstoval
Copy link
Contributor

👍
I'm glad you did not set the menu option in this PR because I think that the menu should not be under design, but directly under the root node.

Everything sounds well to me


->children()
->variableNode('list_actions')
->info('DEPRECATED: use the "actions" option of the "list" view.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we throw some E_USER_DEPRECATED errors instead? Would it be too intrusive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed I think it may be too intrusive.

@javiereguiluz
Copy link
Collaborator Author

@Pierstoval I'm leaving the menu thing until the end on purpose. Meanwhile we'll be able to discuss about it.

@Pierstoval
Copy link
Contributor

Everything seems well, I think you can merge this !

The BC break convenience is really good !

@javiereguiluz javiereguiluz deleted the new_design_global_option branch April 7, 2015 16:08
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.

2 participants