On Mon, Aug 12, 2013 at 12:40 PM, Matevž Bradač <[email protected]> wrote:
> > On 12. Aug, 2013, at 19:08, Olemis Lang wrote: > > > On 8/11/13, Ryan Ollos <[email protected]> wrote: > >> On Sun, Aug 11, 2013 at 8:31 PM, Olemis Lang <[email protected]> wrote: > >> > > [...] > >>>> Which reminds me, I think we should consider moving the Bootstrap > >>>> content > >>>> to `bloodhound_theme`, so I've created #633: > >>>> https://issues.apache.org/bloodhound/ticket/633 > >>>> > >>> > >>> Bootstrap css+js is used by both dashboard and theme . If it is > >>> migrated onto theme how will it be made available for dashboard > >>> layouts and widgets as well ? > >>> > >> > >> The Boostrap CSS and JS are added by bloodhoud_theme.html, so only the > >> paths would need to be changed, e.g. dashboard/css/bootstrap.css -> > >> theme/css/bootstrap.css BloodhoundDashboard calls add_stylesheet a few > >> times, and we'd have to change the path to the content. > >> > > > > my concern is that such approach will lead to cyclic package > > dependencies i.e. theme depends upon dashboard widgets to build the UI > > whereas dashboard depends upon theme for bootstrap styling . That's a > > problematic situation [citation needed]. I do not see the benefit of > > adding such complexity just to move bootstrap files to a different > > plugin (especially if they are working ok where they are now). > > > > The other alternative I see is to create a separate package for shared > assets . > > +1, I think a separate, "core" package would be the correct approach > to deal with functionality/resources common to bloodhound plugins. > > > > >> It seems logical for the Boostrap code to be part of the ThemePlugin > since > >> it is responsible for adding Boostrap to all of the pages (via > >> `bloodhound_theme.html`). I agree with the aim that "Bootstrap-specific > >> enhancements should be added by theme". If more work needs to be done > for > >> this to be realized in BloodhoundDashboard, then I think we should at > least > >> consider making those changes. > >> > > > > Not all bootstrap styles have to be added by theme . Widgets are > > «self-contained» components so they have to take care of loading > > required assets , including bootstrap css+js , no matter on what > > context they are inserted . > > IMO this also speaks in favour of a "core" package. Although the widgets > are self-contained, we probably want to avoid having each widget bring > their > own copies of all the needed resources - this could presumably lead to > various (in)compatibility issues etc. I also like the idea of a core package. The reason I was bothered by having the Bootstrap resources in BloodhoundDashboard is that there doesn't seem to be a logical reason why BloodhoundTheme should have a dependency on BloodhoundDashboard. For new developers this will make it more challenging to understand the structure of the project, and in general having dependencies between the plugins will reduce the ability for users to configure their installations by deactivating Components they aren't interested in. The main question I have is whether that core package should be BloodhoundTheme, or instead we mostly leave BloodhoundTheme the way as it is, and extract the shared resources primarily from BloodhoundDashboard into "BloodhoundCore". It sounds like we are looking to add a new "BloodhoundCore" package.
