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

Reorganise where files are saved #65

Closed
wants to merge 1 commit into from

Conversation

Nerian
Copy link
Contributor

@Nerian Nerian commented Feb 3, 2012

Currently the generated files looks like this:

widgets/parent_widget.rb
widgets/parent/parent_view.html.slim

widgets/parent/child_widget.rb
widgets/parent/child/child_view.html.slim

So in the same folder, we find both view pages from the parent widget and widget code from the child widget.

Unrelated information in the same folder == mess.

I propose to change it to:

widgets/parent/parent_widget.rb
widgets/parent/views/parent_view.html.slim

widgets/parent/child/child_widget.rb
widgets/parent/child/views/child_view.html.slim

…s folder there is a 'views' folder, where its views are located. This makes tree of widgets much more cleaner, since now there is separation between where the code and views are located
@docwhat
Copy link
Contributor

docwhat commented Feb 6, 2012

widgets
├── kaminari
├── panel
│   ├── list
│   │   ├── contents
│   │   │   ├── course
│   │   │   │   └── views
│   │   │   └── views
│   │   ├── header
│   │   │   └── views
│   │   └── views
│   ├── new_course
│   │   └── views
│   └── views
└── users
   └── panel

@docwhat
Copy link
Contributor

docwhat commented Feb 6, 2012

Sorry, that was an example structure give by Nerian. Each 'sub-widget' gets its own directory, which makes it easy to figure out that 'list' is a sub-widget of 'panel' for example.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 6, 2012

This pull request is not complete, it just a discussion starter.

I have changed the generators so that they create the aforementioned structure of folders. There are more things that needs to be changed; for example, apotomo need to be aware of the new location of the views files. I don't know how to change that. Perhaps it need to be done at Cells.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 6, 2012

This is how the proposed structure looks like for a simple 2 levels widget:

Alt Text

And a complex one:

@apotonick
Copy link
Owner

Looks cleaner. This ain't gonna be easy to introduce since it changes the entire assets layout. Maybe we can have a module AlternativeAssets in cells? Why not start a discussion on the mailing list about that, I guess there will be some good opinions there. Can you send a mail to it?

@Nerian
Copy link
Contributor Author

Nerian commented Feb 7, 2012

I think it is better if we just support one way to do it. Less complexity in the long term.

But I agree in that it is a major change. Maybe is time for Cells 4.x.x :)

I'll send an email to the list

@jcquarto
Copy link

jcquarto commented Feb 7, 2012

where would a "layouts" folder go? as per some of the examples on the apotomo.de site, this was always a question anyway....but with this proposed change, it seem it would be a sibling-folder to Views? or a child? And would it be available to that widget and its descendent, only?

@ragrawal
Copy link

ragrawal commented Feb 7, 2012

This is just a half baked suggestion but I am wondering is it possible to have rails like directory structure. Widgets are more like of a controller. So we can have a controller folder inside widgets and a view folder. With view folder there will be different folder for each widget. Generally I hate having very deep file structures and its just make them browsing difficult. Instead we can have some navigation convention to quickly find files. Below is an example of what I am thinking

widget:
controllers
panel_widget.rb
panel_list_widget.rb
....
views
panels
index.html.erb
add.html.erb
remove.js.erb
...
panel_list
update.html.erb
update.js.erb
....

@Nerian
Copy link
Contributor Author

Nerian commented Feb 9, 2012

@jcquarto

We can put layouts inside the views/ folder of each widget. No need to create a subfolder. We can use naming convention like *_layout to differentiate it from normal views.

Layouts inside the view folder of a widget will be just available to that widget. We can have widgets/views/ where global layouts can be stored. Those would be accessible from all widgets.

The widgets/views/ folder would store both layouts and views. Anything that we want to make available for all widgets. For example, views for pagination.

@ragrawal

I prefer a self contained widgets structure instead of spreading every widget on widget/controllers/ and widget/views/. It would be easier to navigate.

@Nerian
Copy link
Contributor Author

Nerian commented Feb 9, 2012

Also, as per the mailing list discussion, we can create css/ and js/ folders for each widget and have it work with Sprokets.

That would be totally epic.

So:

widgets/list_of_courses/
            list_of_courses_widget.rb
            views/
                display.html.slim
                display_layout.html.slim
            javascript/
                display.js.coffee
            stylesheets/
                display.css.sass

widgets/views/                                # Accessible by all widgets
             widget_layout.html.slim     
             a_partial.html.slim

This looks seriously good :)

@Nerian
Copy link
Contributor Author

Nerian commented Mar 5, 2012

@apotonick Thoughts? :)

@wmhobbes
Copy link

I just started working with apotomo these days but this reorganization proposal, including assets, looks much cleaner than the current state. Unless I'm missing something, of course.
Is this still being worked on?

@blazes816
Copy link

I'm a user of Cells, but not Apotomo. Is there any reason this type of change is happening in Apotomo instead of Cells? I'd much prefer this layout, including the asset pipeline integration. I'm looking at starting to make Cells do this, but was wondering if there was a reason you are all looking to do this at the Apotomo level.

@Nerian
Copy link
Contributor Author

Nerian commented Jun 21, 2012

@blazes816 The only reason is that I used Apotomo and not Cells at the time I made this pull request :)

But I agree with you in that this change should be made at Cell's level and then propagate to Apotomo.

@blazes816
Copy link

Alright great, thanks. Just making sure I wasn't missing something. I'll start on making it work in Cells.

@apotonick
Copy link
Owner

Right, this will go to cells. Can anybody explain how we would test the whole assets change?

@Nerian
Copy link
Contributor Author

Nerian commented Jun 22, 2012

Right now is tested like this in Apotomo:

https://github.com/apotonick/apotomo/blob/master/test/rails/widget_generator_test.rb

@apotonick
Copy link
Owner

Yeah but this is just the generator, we also have to make the view finding work, and everything is running ok in a Rails app with asset pipeline etc. That's what I'm asking for.

@blazes816
Copy link

I would imagine the same way assets are tested for Rails core. I'll dig around in their tests and see if I can find anything specific.

@blazes816
Copy link

Here's what I have so far: https://github.com/blazes816/cells/tree/nested_cells

No asset pipeline yet. Just the nested structure.

@blazes816
Copy link

Note that render :partial isn't working. I'm working on figuring that out since it looks like rails handles that directly.

@kristianmandrup
Copy link

I have implemented a similar approach in my fork. The specs pass. The widget generators now support generation of coffeescript or javascript assets and stylesheet stubs for each widget. I would like to compare this with @blazes816 solution to see if we can merge the approaches.

@apotonick
Copy link
Owner

Hey @kristianmandrup I checked your fork and it looks like the alternative view layout is pretty simple to implement. I'd love to do that in cells itself. If we can get your coffeescript/.. generators to work and @blazes816 asset pipeline it would be perfect.

@kristianmandrup
Copy link

Hi @apotonick. Nice to hear that you liked the general idea of my new view layout structure. I agree that the asset pipeline integration with coffee/javascript, styles etc. could be improved and make a for a nice complete package. I think we need a few good cases/examples in order to sanity test any such solution. I haven't looked into @blazes816 work yet. I will be looking more into it later in the week. Feel free to grab what ever you find useful from my fork. Cheers!

@kuraga
Copy link
Contributor

kuraga commented Mar 30, 2013

Hello! Why don't you consider more Rails-way: app/widgets, app/widgets_views?

@kristianmandrup
Copy link

Yes, that was also an option I thought about. The only problem with this design, is that it makes it harder to move around a widget, fx changing the namespace. Then you have to synchronize within two separate folders.

app/widgets/controllers and app/widgets/views might be an idea? why not have helpers, decorators or even commands also? Lot's of options!

@kuraga
Copy link
Contributor

kuraga commented Mar 30, 2013

+1 to app/widgets/controllers and app/widgets/views. And routes.rb :-)

|31.03.2013 00:03, Kristian Mandrup пишет:

Yes, that was also an option I thought about. The only problem with
this design, is that it makes it harder to move around a widget, fx
changing the namespace. Then you have to synchronize within two
separate folders.

|app/widgets/controllers| and |app/widgets/views| might be an idea?
why not have helpers, decorators or even commands also? Lot's of options!


Reply to this email directly or view it on GitHub
#65 (comment).

@apotonick
Copy link
Owner

I'd love to have widgets as a self-contained folder to make reusability across projects as simple as possible. Something like

app/widgets/comments
app/widgets/comments/comment_widget.rb
app/widgets/comments/views/..
app/widgets/comments/helpers/..           

Or is that stupid? I guess Kristian proposed that a long time ago and I promised him to write it (also, a long time ago) ;-)

@kuraga
Copy link
Contributor

kuraga commented Apr 2, 2013

Yes, he did it, and I should recreate this patch (it's our agreement :-)). But I think it should be rewritten for Cells...

@kristianmandrup
Copy link

Yes, that was the alternative approach - breaking the "rails structure convention", which makes it easy to move everything belonging to one widget in one go, like changing internal namespace structure, without having to synchronize in various disparate places!!!

I also believe it belongs more in cells? perhaps next major version (major breaking changes!)

@apotonick
Copy link
Owner

It should be in cells, right. We could introduce a switch where you can have the old behavior back. Now we need to discuss the structure thoroughly before we implement it. BTW- I don't see a problem with breaking Rails' convention here as I am not a big fan of the global directories at all!

@kristianmandrup
Copy link

👍

@kuraga
Copy link
Contributor

kuraga commented Sep 24, 2013

Some news?

@apotonick
Copy link
Owner

This is now implemented in Cells: trailblazer/cells@25b65d1

@apotonick apotonick closed this Apr 10, 2014
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.

None yet

9 participants