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

support configuration of individual view paths #6844

Closed
ewinslow opened this Issue May 15, 2014 · 14 comments

Comments

Projects
None yet
5 participants
@ewinslow
Member

ewinslow commented May 15, 2014

Use case: third party assets

Now that we support static assets as views, it would be uber cool to also be able to specify that a file or files in the vendors directory should be the content for a particular view. That way we don't have to copy third party assets into the views system to get the benefits of the view conventions.

set_view_location only works on directories, not individual views, iirc.

possible API:

<?php
// /views.php
return [
  'default' => [
    'js/my/lib.js' => __DIR__ . "/bower_components/mylib/mylib.js",
  ],
];
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay May 15, 2014

Member

👍

Member

mrclay commented May 15, 2014

👍

@brettp

This comment has been minimized.

Show comment
Hide comment
@brettp

brettp May 15, 2014

Member

I like the idea, and have always thought set_view_location() was a confusing function. I'm not sure how I feel about returning an array of options from the view, though. Do you prefer this over configuration for consistency within the views system?

Member

brettp commented May 15, 2014

I like the idea, and have always thought set_view_location() was a confusing function. I'm not sure how I feel about returning an array of options from the view, though. Do you prefer this over configuration for consistency within the views system?

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 May 15, 2014

Member

Are you suggesting changing the usage of set_view_location() to this?

    $options = array(
        'default' => array(
            'js/my/lib.js' => __DIR__ . '/bower_components/mylib/mylib.js',
        )
    );

    set_view_location($options);
Member

beck24 commented May 15, 2014

Are you suggesting changing the usage of set_view_location() to this?

    $options = array(
        'default' => array(
            'js/my/lib.js' => __DIR__ . '/bower_components/mylib/mylib.js',
        )
    );

    set_view_location($options);
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay May 15, 2014

Member

@beck24 I'm 99% sure he's referring to a config file in the root of the plugin. Declarative configuration.

Member

mrclay commented May 15, 2014

@beck24 I'm 99% sure he's referring to a config file in the root of the plugin. Declarative configuration.

@beck24

This comment has been minimized.

Show comment
Hide comment
@beck24

beck24 May 15, 2014

Member

ok, that makes more sense

Member

beck24 commented May 15, 2014

ok, that makes more sense

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow May 18, 2014

Member

@brettp -- Currently I have a preference for return-the-config style files in general, even though a lot of our config is global function calls (elgg_register_action, elgg_register_plugin_hook, etc.). The only place we do this right now is with languages and I find that to be quite a compelling experience that eliminates redundancy well (elgg_register_language_key over and over would be a little boring/tedious).

Member

ewinslow commented May 18, 2014

@brettp -- Currently I have a preference for return-the-config style files in general, even though a lot of our config is global function calls (elgg_register_action, elgg_register_plugin_hook, etc.). The only place we do this right now is with languages and I find that to be quite a compelling experience that eliminates redundancy well (elgg_register_language_key over and over would be a little boring/tedious).

@ewinslow ewinslow changed the title from feature(views): support configuration of individual view paths to support configuration of individual view paths Jul 16, 2014

@ewinslow ewinslow self-assigned this Jul 16, 2014

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow Jul 16, 2014

Member

So there is an obvious use-case I hadn't considered at first of registering a whole directory of views, but at a custom base. jQuery, for example, provides its AMD source in the bower directory, but you don't want to have to either:

  • individually register each module as a view
  • edit the bower install to be structured like a views directory (too much hassle on update)

Here's my proposed API for addressing the issue:

elgg_register_views('js/jquery', __DIR__ . "/bower_components/jquery/src", "default");

This would recursively register everything the src directory, so you'd have views like:

  • js/jquery/deferred.js (points to bower_components/jquery/src/deferred.js)
  • js/jquery/core/ready.js (points to bower_components/jquery/src/core/ready.js)

The single view registration would be similar:

elgg_register_view('js/jquery.js', __DIR__ . "/bower_components/jquery/jquery.min.js", "default");

For now I think this is preferable to a configuration file in the root of the plugins directory for consistency with the other registration functions and because it might be too confusing to have a single configuration file that does both single view registration and recursive view registration.

Member

ewinslow commented Jul 16, 2014

So there is an obvious use-case I hadn't considered at first of registering a whole directory of views, but at a custom base. jQuery, for example, provides its AMD source in the bower directory, but you don't want to have to either:

  • individually register each module as a view
  • edit the bower install to be structured like a views directory (too much hassle on update)

Here's my proposed API for addressing the issue:

elgg_register_views('js/jquery', __DIR__ . "/bower_components/jquery/src", "default");

This would recursively register everything the src directory, so you'd have views like:

  • js/jquery/deferred.js (points to bower_components/jquery/src/deferred.js)
  • js/jquery/core/ready.js (points to bower_components/jquery/src/core/ready.js)

The single view registration would be similar:

elgg_register_view('js/jquery.js', __DIR__ . "/bower_components/jquery/jquery.min.js", "default");

For now I think this is preferable to a configuration file in the root of the plugins directory for consistency with the other registration functions and because it might be too confusing to have a single configuration file that does both single view registration and recursive view registration.

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow Jul 27, 2014

Member

elgg_register_view and elgg_register_views feel too closely named now. I suspect it would be begging for typos/confusion. How about:

  • elgg_register_view_file($name, $path, $viewtype = 'default') {}
  • elgg_register_views_dir($prefix, $path, $viewtype = 'default') {}

To maintain performance, these probably also should be noops if the viewpath cache is in use. We don't want plugins walking their vendors directories on every page load.

Member

ewinslow commented Jul 27, 2014

elgg_register_view and elgg_register_views feel too closely named now. I suspect it would be begging for typos/confusion. How about:

  • elgg_register_view_file($name, $path, $viewtype = 'default') {}
  • elgg_register_views_dir($prefix, $path, $viewtype = 'default') {}

To maintain performance, these probably also should be noops if the viewpath cache is in use. We don't want plugins walking their vendors directories on every page load.

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jul 30, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Aug 6, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Aug 7, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Aug 18, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Sep 8, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Sep 8, 2014

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 25, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 25, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 25, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 25, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 25, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 26, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 26, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 26, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 26, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jan 26, 2015

@jdalsem

This comment has been minimized.

Show comment
Hide comment
@jdalsem

jdalsem Jan 26, 2015

Member

i think these absolute paths provide more features than thought of before; for instance you could have online editing of view files (but store them in dataroot), or you can have cached version of the views, just by pointing the view location to the correct files... very interesting change!

Member

jdalsem commented Jan 26, 2015

i think these absolute paths provide more features than thought of before; for instance you could have online editing of view files (but store them in dataroot), or you can have cached version of the views, just by pointing the view location to the correct files... very interesting change!

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow Jan 26, 2015

Member

Yes, I'm excited about the new possibilities, but trying to be diligent about adding auto-tests wherever possible to cover all features that we didn't have tests for before, so it is taking some time to be thorough.

Member

ewinslow commented Jan 26, 2015

Yes, I'm excited about the new possibilities, but trying to be diligent about adding auto-tests wherever possible to cover all features that we didn't have tests for before, so it is taking some time to be thorough.

ewinslow added a commit to ewinslow/Elgg that referenced this issue Feb 16, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

chore(views): refactor views service to store absolute paths
Refs Elgg#6844

Conflicts:
	engine/classes/Elgg/ViewsService.php
	engine/tests/phpunit/Elgg/Di/ServiceProviderTest.php
	engine/tests/phpunit/bootstrap.php

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

chore(views): refactor views service to store absolute paths
Refs Elgg#6844

Conflicts:
	engine/classes/Elgg/ViewsService.php
	engine/tests/phpunit/Elgg/Di/ServiceProviderTest.php
	engine/tests/phpunit/bootstrap.php
@mrclay

This comment has been minimized.

Show comment
Hide comment
@mrclay

mrclay May 29, 2015

Member
elgg_register_views_prefix_dir('js/jquery', __DIR__ . "/bower_components/jquery/src", "default");

FWIW this seems implementable without a huge rewrite (seems a bit risky for 2.0 at this point). getViewLocation(), before returning the core views dir, would check the viewname against the list of prefixes.

Member

mrclay commented May 29, 2015

elgg_register_views_prefix_dir('js/jquery', __DIR__ . "/bower_components/jquery/src", "default");

FWIW this seems implementable without a huge rewrite (seems a bit risky for 2.0 at this point). getViewLocation(), before returning the core views dir, would check the viewname against the list of prefixes.

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

chore(views): refactor views service to store absolute paths
Refs Elgg#6844

Conflicts:
	engine/classes/Elgg/ViewsService.php
	engine/tests/phpunit/Elgg/Di/ServiceProviderTest.php
	engine/tests/phpunit/bootstrap.php

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

chore(views): refactor views service to store absolute paths
Refs Elgg#6844

Conflicts:
	engine/classes/Elgg/ViewsService.php
	engine/tests/phpunit/Elgg/Di/ServiceProviderTest.php
	engine/tests/phpunit/bootstrap.php

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue May 29, 2015

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue May 29, 2015

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow May 30, 2015

Member

I think that depends when we want our freeze date to happen. There's still a lot of like to get in for 2.0... And I have other ideas of what is like to see happen for 2.0 regarding views system. The rewrite makes those easier too.

Member

ewinslow commented May 30, 2015

I think that depends when we want our freeze date to happen. There's still a lot of like to get in for 2.0... And I have other ideas of what is like to see happen for 2.0 regarding views system. The rewrite makes those easier too.

@ewinslow

This comment has been minimized.

Show comment
Hide comment
@ewinslow

ewinslow May 30, 2015

Member

I.e. aliasing js/{view} and js/{view}.js to {view}.js. Similarly for CSS.

Member

ewinslow commented May 30, 2015

I.e. aliasing js/{view} and js/{view}.js to {view}.js. Similarly for CSS.

@mrclay

This comment has been minimized.

Show comment
Hide comment
Member

mrclay commented May 30, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jun 8, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jun 8, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jun 9, 2015

ewinslow added a commit to ewinslow/Elgg that referenced this issue Jun 12, 2015

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 17, 2015

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 17, 2015

feature(views): Allow specifying exact view paths via views.php
This also uses this feature for core to link views to vendor(s) files.

Fixes Elgg#6844

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 17, 2015

feature(views): Allow specifying exact view paths via views.php
This also uses this feature for core to link views to vendor(s) files.

Fixes Elgg#6844

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 18, 2015

feature(views): Allow specifying exact view paths via views.php
We use this feature to bring vendor(s) files into the views system.
This also loads jQuery UI completely asynchronously.

Fixes Elgg#6844
Fixes Elgg#8515

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 18, 2015

feature(views): Allow specifying exact view paths via views.php
We use this feature to bring vendor(s) files into the views system.
This also loads jQuery UI completely asynchronously.

Fixes Elgg#6844
Fixes Elgg#8515

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 18, 2015

feature(views): Allow specifying exact view paths via views.php
This also uses this feature for core to link views to vendor(s) files.

Fixes Elgg#6844

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 20, 2015

feature(views): Allow specifying exact view paths via views.php
We use this feature to bring vendor(s) files into the views system.
This also loads jQuery UI completely asynchronously.

Fixes Elgg#6844
Fixes Elgg#8515

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 20, 2015

feature(views): Allow specifying exact view paths via views.php
We use this feature to bring vendor(s) files into the views system.
This also loads jQuery UI completely asynchronously.

Fixes Elgg#6844
Fixes Elgg#8515
Fixes Elgg#8527

mrclay added a commit to mrclay/Elgg-leaf that referenced this issue Jun 20, 2015

feature(views): Allow specifying exact view paths via views.php
We use this feature to bring vendor(s) files into the views system.
This also loads jQuery UI completely asynchronously.

Fixes Elgg#6844
Fixes Elgg#8515
Fixes Elgg#8527
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment