You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by as...@apache.org on 2013/07/23 13:50:20 UTC
svn commit: r1505992 -
/bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Author: astaric
Date: Tue Jul 23 11:50:20 2013
New Revision: 1505992
URL: http://svn.apache.org/r1505992
Log:
Value of default combo in product admin should be product.prefix, not product.name.
Modified:
bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Modified: bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html?rev=1505992&r1=1505991&r2=1505992&view=diff
==============================================================================
--- bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html (original)
+++ bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html Tue Jul 23 11:50:20 2013
@@ -142,7 +142,7 @@
</td>
<td class="owner">$prod.owner</td>
<td class="default">
- <input type="radio" name="default" value="$prod.name"
+ <input type="radio" name="default" value="$prod.prefix"
checked="${prod.name==default or None}" />
</td>
</tr>
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Ryan Ollos <ry...@wandisco.com>.
On Mon, Aug 12, 2013 at 12:40 PM, Matevž Bradač <ma...@digiverse.si> wrote:
>
> On 12. Aug, 2013, at 19:08, Olemis Lang wrote:
>
> > On 8/11/13, Ryan Ollos <ry...@wandisco.com> wrote:
> >> On Sun, Aug 11, 2013 at 8:31 PM, Olemis Lang <ol...@gmail.com> 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.
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/15/13, Olemis Lang <ol...@gmail.com> wrote:
> On 8/15/13, Ryan Ollos <ry...@wandisco.com> wrote:
>> On Mon, Aug 12, 2013 at 4:09 PM, Olemis Lang <ol...@gmail.com> wrote:
> [...]
>>>
>>> in Trac it seems that
>>> href generation with protocol off is buggy , so I always end up
>>> including it ...
>>>
>>
>> Is it something we could push a fix for back to Trac for the 1.0.2
>> release?
>>
>
> hmmm ... let's see .
>
http://trac.edgewall.org/ticket/11277#comment:2
;)
--
Regards,
Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/15/13, Ryan Ollos <ry...@wandisco.com> wrote:
> On Mon, Aug 12, 2013 at 4:09 PM, Olemis Lang <ol...@gmail.com> wrote:
[...]
>>
>> in Trac it seems that
>> href generation with protocol off is buggy , so I always end up
>> including it ...
>>
>
> Is it something we could push a fix for back to Trac for the 1.0.2 release?
>
hmmm ... let's see .
--
Regards,
Olemis - @olemislc
Apache™ Bloodhound contributor
http://issues.apache.org/bloodhound
http://blood-hound.net
Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/
Featured article:
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Ryan Ollos <ry...@wandisco.com>.
On Mon, Aug 12, 2013 at 4:09 PM, Olemis Lang <ol...@gmail.com> wrote:
> On 8/12/13, Matevž Bradač <ma...@digiverse.si> wrote:
> > On 12. Aug, 2013, at 19:08, Olemis Lang wrote:
> >
> [...]
> >
> >>> 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.
> >
>
> Yes . Even if using add_(script|stylesheet) if the the same lib code
> is loaded from different locations then it will be executed twice ...
> definitely triggering errors , not to mention the degraded performance
> due to download overhead .
>
> For JS however there's yet another solution . What I do in practice in
> my plugins is similar to Trac's jquery_location i.e.
>
> - declare an option e.g. [trac] bootstrap
>
That seems like a useful enhancement for Bloodhound.
> - set its default value to load it from a well know CDN location
> * e.g. my preferred option is Cloudflare's CDNJS , the value
> would be like this
> //cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/2.3.1
> - URLs to individual files would be generated from there .
> - All components will read that option , it does not really matter
> where exactly it is defined as long as it will be read from the
> same location .
>
> The only drawbacks with this approach is that in Trac it seems that
> href generation with protocol off is buggy , so I always end up
> including it ...
>
Is it something we could push a fix for back to Trac for the 1.0.2 release?
> [...]
>
> --
> Regards,
>
> Olemis - @olemislc
>
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/12/13, Matevž Bradač <ma...@digiverse.si> wrote:
> On 12. Aug, 2013, at 19:08, Olemis Lang wrote:
>
[...]
>
>>> 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.
>
Yes . Even if using add_(script|stylesheet) if the the same lib code
is loaded from different locations then it will be executed twice ...
definitely triggering errors , not to mention the degraded performance
due to download overhead .
For JS however there's yet another solution . What I do in practice in
my plugins is similar to Trac's jquery_location i.e.
- declare an option e.g. [trac] bootstrap
- set its default value to load it from a well know CDN location
* e.g. my preferred option is Cloudflare's CDNJS , the value
would be like this
//cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/2.3.1
- URLs to individual files would be generated from there .
- All components will read that option , it does not really matter
where exactly it is defined as long as it will be read from the
same location .
The only drawbacks with this approach is that in Trac it seems that
href generation with protocol off is buggy , so I always end up
including it ...
[...]
--
Regards,
Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Matevž Bradač <ma...@digiverse.si>.
On 12. Aug, 2013, at 19:08, Olemis Lang wrote:
> On 8/11/13, Ryan Ollos <ry...@wandisco.com> wrote:
>> On Sun, Aug 11, 2013 at 8:31 PM, Olemis Lang <ol...@gmail.com> 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.
>
> --
> Regards,
>
> Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/11/13, Ryan Ollos <ry...@wandisco.com> wrote:
> On Sun, Aug 11, 2013 at 8:31 PM, Olemis Lang <ol...@gmail.com> 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 .
> 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 .
--
Regards,
Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Ryan Ollos <ry...@wandisco.com>.
On Sun, Aug 11, 2013 at 8:31 PM, Olemis Lang <ol...@gmail.com> wrote:
> On 8/11/13, Ryan Ollos <ry...@wandisco.com> wrote:
> > On Thu, Aug 8, 2013 at 9:32 AM, Olemis Lang <ol...@gmail.com> wrote:
> >
> >> On 8/8/13, Ryan Ollos <ry...@wandisco.com> wrote:
> >> > On Tue, Jul 23, 2013 at 4:53 AM, Anze Staric <an...@gmail.com>
> >> wrote:
> >> >
> >> >> Why are there two templates for product admin
> >> >> (bloodhound_multiproduct/multiproduct/templates/admin_products.html
> >> >> and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is
> the
> >> >> first one ever used?
> >> >
> >> >
> >> > I'm not exactly sure, but I'd previously thought that
> >> `admin_products.html`
> >> > exists so that the MultiProduct plugin will work when the Bloodhound
> >> Theme
> >> > is disabled.
> >> >
> >>
> >> right ... since the beginning Gary suggested that Bootstrap-specific
> >> enhancements should be added by theme , and simple templates were
> >> provided in multiproduct plugin .
> >>
> >
> > 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.
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.
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/11/13, Ryan Ollos <ry...@wandisco.com> wrote:
> On Thu, Aug 8, 2013 at 9:32 AM, Olemis Lang <ol...@gmail.com> wrote:
>
>> On 8/8/13, Ryan Ollos <ry...@wandisco.com> wrote:
>> > On Tue, Jul 23, 2013 at 4:53 AM, Anze Staric <an...@gmail.com>
>> wrote:
>> >
>> >> Why are there two templates for product admin
>> >> (bloodhound_multiproduct/multiproduct/templates/admin_products.html
>> >> and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is the
>> >> first one ever used?
>> >
>> >
>> > I'm not exactly sure, but I'd previously thought that
>> `admin_products.html`
>> > exists so that the MultiProduct plugin will work when the Bloodhound
>> Theme
>> > is disabled.
>> >
>>
>> right ... since the beginning Gary suggested that Bootstrap-specific
>> enhancements should be added by theme , and simple templates were
>> provided in multiproduct plugin .
>>
>
> 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 ?
[...]
--
Regards,
Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Ryan Ollos <ry...@wandisco.com>.
On Thu, Aug 8, 2013 at 9:32 AM, Olemis Lang <ol...@gmail.com> wrote:
> On 8/8/13, Ryan Ollos <ry...@wandisco.com> wrote:
> > On Tue, Jul 23, 2013 at 4:53 AM, Anze Staric <an...@gmail.com>
> wrote:
> >
> >> Why are there two templates for product admin
> >> (bloodhound_multiproduct/multiproduct/templates/admin_products.html
> >> and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is the
> >> first one ever used?
> >
> >
> > I'm not exactly sure, but I'd previously thought that
> `admin_products.html`
> > exists so that the MultiProduct plugin will work when the Bloodhound
> Theme
> > is disabled.
> >
>
> right ... since the beginning Gary suggested that Bootstrap-specific
> enhancements should be added by theme , and simple templates were
> provided in multiproduct plugin .
>
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
If this were to be done, it doesn't need to be done soon, I just wanted to
have a ticket for it. Maybe before 1.0 release though.
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Olemis Lang <ol...@gmail.com>.
On 8/8/13, Ryan Ollos <ry...@wandisco.com> wrote:
> On Tue, Jul 23, 2013 at 4:53 AM, Anze Staric <an...@gmail.com> wrote:
>
>> Why are there two templates for product admin
>> (bloodhound_multiproduct/multiproduct/templates/admin_products.html
>> and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is the
>> first one ever used?
>
>
> I'm not exactly sure, but I'd previously thought that `admin_products.html`
> exists so that the MultiProduct plugin will work when the Bloodhound Theme
> is disabled.
>
right ... since the beginning Gary suggested that Bootstrap-specific
enhancements should be added by theme , and simple templates were
provided in multiproduct plugin .
--
Regards,
Olemis - @olemislc
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Ryan Ollos <ry...@wandisco.com>.
On Tue, Jul 23, 2013 at 4:53 AM, Anze Staric <an...@gmail.com> wrote:
> Why are there two templates for product admin
> (bloodhound_multiproduct/multiproduct/templates/admin_products.html
> and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is the
> first one ever used?
I'm not exactly sure, but I'd previously thought that `admin_products.html`
exists so that the MultiProduct plugin will work when the Bloodhound Theme
is disabled.
Re: svn commit: r1505992 - /bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
Posted by Anze Staric <an...@gmail.com>.
Why are there two templates for product admin
(bloodhound_multiproduct/multiproduct/templates/admin_products.html
and bloodhound_theme/bhtheme/templates/bh_admin_products.html)? Is the
first one ever used?
On Tue, Jul 23, 2013 at 1:50 PM, <as...@apache.org> wrote:
> Author: astaric
> Date: Tue Jul 23 11:50:20 2013
> New Revision: 1505992
>
> URL: http://svn.apache.org/r1505992
> Log:
> Value of default combo in product admin should be product.prefix, not product.name.
>
> Modified:
> bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
>
> Modified: bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html
> URL: http://svn.apache.org/viewvc/bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html?rev=1505992&r1=1505991&r2=1505992&view=diff
> ==============================================================================
> --- bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html (original)
> +++ bloodhound/trunk/bloodhound_theme/bhtheme/templates/bh_admin_products.html Tue Jul 23 11:50:20 2013
> @@ -142,7 +142,7 @@
> </td>
> <td class="owner">$prod.owner</td>
> <td class="default">
> - <input type="radio" name="default" value="$prod.name"
> + <input type="radio" name="default" value="$prod.prefix"
> checked="${prod.name==default or None}" />
> </td>
> </tr>
>
>