You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bloodhound.apache.org by Apache Bloodhound <bl...@incubator.apache.org> on 2012/05/25 02:20:05 UTC
[Apache Bloodhound] #88: Generic conversions of tables to bootstrap
#88: Generic conversions of tables to bootstrap
-------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Keywords: |
-------------------------+-----------------
Given that we do not yet have complete coverage of templates, and it would
be difficult to include coverage of all plugin templates, I would like to
explore the possibility of providing relatively simple rules to adapt
templates for bootstrap.
This ticket looks at converting tables as particular elements that need to
be made prettier on removal of the trac styles.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
Why not just to use some ''javascript'' (''jquery'') code adding `.table`
class dynamically ? ... e.g. ...
{{{
#!js
$('table').addClass('table')
}}}
There's another question : is it ok to convert automatically all tables to
bootstrap styling ?
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:2>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
This third message is more like an assessment of (afaics) both solutions.
1. It seems approaches mentioned above do not transform tables
generated using WikiFormatting e.g.
{{{
|| || col 0 || col 1 ||
|| row 0 || cell 0 , 0 || cell 1 , 0 ||
}}}
2. It'd be nice if we could review this ''XPath'' expression
`body//table[not(contains(@class, 'table'))]` . It seems to me
that e.g. ''HTML'' table shown below will not match even if it should
''';)'''
{{{
#!xml
<table class="wiki prettytable">
<tr>
<td> </td>
<td> col 0 </td>
<td> col 1 </td>
</tr>
<tr>
<td> row 0 </td>
<td> cell 0 , 0 </td>
<td> cell 1 , 0 </td>
</tr>
</table>
}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:7>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+--------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: closed
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: fixed | Keywords:
--------------------------+--------------------
Comment (by olemis):
Replying to [comment:14 gjm]:
> Replying to [comment:13 olemis]:
>
[...]
>
> I understood that for those selectors the rules are treated as being of
equal specificity and so it is the last css rule in the cascade that sets
the property. I would therefore expect the two tables to have the same
{{{background-color}}} regardless of the difference in the order of the
classes in the elements.
>
> Of course, to confirm this to myself I have tested based on your
example. Did you not also find this? Is there a browser which does enforce
order dependence?
You are right . This should be ok , so I'm in favor of keeping this ticket
closed .
''';)'''
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:17>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+--------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: closed
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: fixed | Keywords:
--------------------------+--------------------
Changes (by gjm):
* status: new => closed
* resolution: => fixed
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:16>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
r1343821 provides a mechanism to filter genshi streams to add css classes
r1343847 adds the conversion for tables
If I haven't missed anything obvious, I would like to suggest the
following as being a little more obvious about what it is doing.
{{{
#!python
def attr_modifier(name, event):
attrs = event[1][1]
class_list = attrs.get(name, '').split()
self.log.debug('BH Theme : Element classes ' + str(class_list))
class_list += classes
class_list = sorted(set(class_list))
out_classes = ' '.join(class_list)
self.log.debug('BH Theme : Inserting class ' + out_classes)
return out_classes
return attr_modifier
}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:9>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to bootstrap
Posted by Gary <ga...@wandisco.com>.
Hi,
I have some more suggestions for how we might provide limited support
for plugin templates to allow them to blend in a little easier with our
theme.
https://issues.apache.org/bloodhound/ticket/88 contains the initial
suggestion, including a helper function to help add classes to elements
and applying it to tables.
https://issues.apache.org/bloodhound/ticket/36 begins to look at buttons
with the same approach.
If this idea is seen as having merit and appears to work then further
tickets should be raised for other predictable conversions.
Cheers,
Gary
On 25/05/12 01:29, Apache Bloodhound wrote:
> #88: Generic conversions of tables to bootstrap
> --------------------------+-----------------
> Reporter: gjm | Owner: gjm
> Type: enhancement | Status: new
> Priority: major | Milestone:
> Component: dashboard | Version:
> Resolution: | Keywords:
> --------------------------+-----------------
>
> Comment (by gjm):
>
> Part of the barrier to doing this is a simple way of adding the bootstrap
> classes to objects - it is useful to make this fairly simple so that it is
> reusable for other conversions. The following might suffice as a starting
> point.
>
> {{{
> #!diff
> Index: trunk/bloodhound_theme/bhtheme/theme.py
> ===================================================================
> --- trunk/bloodhound_theme/bhtheme/theme.py (revision 1341781)
> +++ trunk/bloodhound_theme/bhtheme/theme.py (working copy)
> @@ -105,6 +106,21 @@
> def post_process_request(self, req, template, data, content_type):
> """Post process request filter.
> Removes all trac provided css if required"""
> + def add_classes(attributes,*args):
> + """Helper function to add classes to the existing attributes
> found
> + by a select('@*') or similar. Example use:
> +<element py:attrs="add_classes(select('@*'), 'cl1', 'cl2')"/>
> + """
> + attrib_list = list(attributes)
> + if attrib_list:
> + attrs = dict(list(attrib_list[0]))
> + else:
> + attrs = {}
> + if len(args):
> + attrs['class'] = ' '.join((attrs.get('class',''),) +
> args)
> + return attrs
> + data['add_classes'] = add_classes
> +
> def is_active_theme():
> is_active = False
> active_theme = ThemeEngineSystem(self.env).theme
> }}}
>
> Using this, it is then possible to transform unconverted tables with
> something like this:
>
> {{{
> #!diff
> Index: trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
> ===================================================================
> --- trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
> (revision 1341816)
> +++ trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
> (working copy)
> @@ -252,4 +252,30 @@
> </body></py:match>
>
> <xi:include href="bh_admin.html" py:with="bh_fix_legacy = True"/>
> +
> +<py:match path="table[not(contains(@class,'table'))]" once="false"
> buffer="true">
> +<table py:attrs="add_classes(select('@*'), 'table', 'table-striped',
> 'table-bordered', 'table-condensed')">
> + ${select('*|text()|comment()')}
> +</table>
> +</py:match>
> +
> </html>
> }}}
>
> Similar conversions could be applied for buttons and other common
> interface pieces but these should be considered in separate tickets.
>
> Comments appreciated.
>
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
Part of the barrier to doing this is a simple way of adding the bootstrap
classes to objects - it is useful to make this fairly simple so that it is
reusable for other conversions. The following might suffice as a starting
point.
{{{
#!diff
Index: trunk/bloodhound_theme/bhtheme/theme.py
===================================================================
--- trunk/bloodhound_theme/bhtheme/theme.py (revision 1341781)
+++ trunk/bloodhound_theme/bhtheme/theme.py (working copy)
@@ -105,6 +106,21 @@
def post_process_request(self, req, template, data, content_type):
"""Post process request filter.
Removes all trac provided css if required"""
+ def add_classes(attributes,*args):
+ """Helper function to add classes to the existing attributes
found
+ by a select('@*') or similar. Example use:
+ <element py:attrs="add_classes(select('@*'), 'cl1', 'cl2')"/>
+ """
+ attrib_list = list(attributes)
+ if attrib_list:
+ attrs = dict(list(attrib_list[0]))
+ else:
+ attrs = {}
+ if len(args):
+ attrs['class'] = ' '.join((attrs.get('class',''),) +
args)
+ return attrs
+ data['add_classes'] = add_classes
+
def is_active_theme():
is_active = False
active_theme = ThemeEngineSystem(self.env).theme
}}}
Using this, it is then possible to transform unconverted tables with
something like this:
{{{
#!diff
Index: trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
===================================================================
--- trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
(revision 1341816)
+++ trunk/bloodhound_theme/bhtheme/templates/bloodhound_theme.html
(working copy)
@@ -252,4 +252,30 @@
</body></py:match>
<xi:include href="bh_admin.html" py:with="bh_fix_legacy = True"/>
+
+ <py:match path="table[not(contains(@class,'table'))]" once="false"
buffer="true">
+ <table py:attrs="add_classes(select('@*'), 'table', 'table-striped',
'table-bordered', 'table-condensed')">
+ ${select('*|text()|comment()')}
+ </table>
+ </py:match>
+
</html>
}}}
Similar conversions could be applied for buttons and other common
interface pieces but these should be considered in separate tickets.
Comments appreciated.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:1>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
Replying to [comment:5 olemis]:
> - In mockups e.g.
[http://svn.apache.org/repos/asf/incubator/bloodhound/trunk/doc/html-
templates/dashboard.html Dashboard]
> there are tables not having every Bootstrap CSS class applied .
Therefore
> I suggest to only add `.table` (... and maybe `table-condensed`
''';)''' by
> default , instead of applying them all .
Well, it was only an example. The main thing is that we decide on a common
look so that only those tables that are known to require a different look
will be required to specify their classes.
> - IMO there are some tables that should not even have `.table` class .
> One such example is side-by-side diff view .
>
If this is the case and appropriate styling is available then we only need
a means to distinguish them as tables that should not be processed in this
way.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:8>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
Yes, we could do this with jQuery instead and I would like for us to
decide the best approach. At the moment our interface appears to rely
somewhat on javascript as things like the main menu's 'Apps' drop-down
requires it to display the additional main menu links. If we are not going
to consider strategies to deal with that then it might not be worth
considering anything other than a jQuery approach here.
The advantages of using jQuery would obviously be moving the processing to
the client side, and features like the selection of elements and the
adding and removing of attributes would be more familiar to developers
(and we would not need to maintain the python helper functions to
manipulate attributes).
However, as the classes are added to adjust the presentation and not
enable javascript based functionality, it would not be normal for such
presentation to be dependent on whether javascript is enabled.
As for the second question, automatic conversion of all tables might make
sense if we agree that all tables have a specific set of classes or it is
easy to identify those that should not. Note that the xpath that I specify
avoids matching tables with the table class already applied for instance.
Also, it should be noted that this whole discussion at this point is
dependent on the assumption that table elements in general will not be
abused and used for anything other than displaying tabular data.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:3>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
I'll dedicate this first message to talk about whether automatic
conversion of all tables to bootstrap styling is the right way to go . We
need to consider that :
- In mockups e.g.
[http://svn.apache.org/repos/asf/incubator/bloodhound/trunk/doc/html-
templates/dashboard.html Dashboard]
there are tables not having every Bootstrap CSS class applied .
Therefore
I suggest to only add `.table` (... and maybe `table-condensed`
''';)''' by
default , instead of applying them all .
- IMO there are some tables that should not even have `.table` class .
One such example is side-by-side diff view .
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:5>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
Ok . There's even yet another approach to get this done , so I put it
under your consideration as well . It relies on stream filters and is
similar to what Trac does since long time ago in order to insert
'''automatically''' form token for ''HTTP'' ''POST'' requests. I think I
better submit a patch illustrating what it will look like ...
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:4>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
OK.. in principle, someone could be using a CSS rule like
{{{
#!css
TABLE[class*="foo bar"] {
width: 50%;
}
}}}
Strictly speaking this is not order dependant as it would not match <table
class="foo baz bar"> but it illustrates the same problem.
I am still not convinced that we should support this as there is no
particular excuse for requiring a specific order when you can just apply
another class instead. Using {{{TABLE.dirlist}}} style selectors, there is
no order dependency on the classes that I am aware of.
I would be all for simplifying to accept the order decided by the set
type. However, if it is insisted, I would suggest the following as a bit
more readable while there is a lack of a standard ordered set type:
{{{
#!python
from collections import OrderedDict
class_list = OrderedDict((k, None) for k in class_list).keys()
out_classes = ' '.join(class_list)
}}}
That is just my preference of course but I do want to emphasise that code
readability is very important.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:11>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
Replying to [comment:13 olemis]:
> I was thinking more about the following situation
>
> rule
>
> {{{
> #!css
>
> .class2 {
> background-color: #FFFFCC;
> }
> }}}
>
> ... and ...
>
> {{{
> #!css
>
> .class1 {
> background-color: #FF0000;
> }
> }}}
>
> ... and these two tables ...
>
> {{{
> #!xml
>
> <table class="class1 class2" >
> <!-- Whatever ... -->
> </table>
>
> <table class="class2 class1">
> <!-- Whatever ... -->
> </table>
>
> }}}
[...]
> If classes are sorted by name then both tables in example above will
look the same , and maybe they should not ... In order to prevent these
side-effects from happening , class order should be preserved ''';)'''
I understood that for those selectors the rules are treated as being of
equal specificity and so it is the last css rule in the cascade that sets
the property. I would therefore expect the two tables to have the same
{{{background-color}}} regardless of the difference in the order of the
classes in the elements.
Of course, to confirm this to myself I have tested based on your example.
Did you not also find this? Is there a browser which does enforce order
dependence?
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:14>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
As {{{tokenize}}} doesn't seem to be available, I believe that this should
be an improvement for matching the the lack of the table class (adapted
from [http://dubinko.info/blog/2007/10/01/simple-parsing-of-space-
seprated-attributes-in-xpathxslt/ this link]):
{{{
#!diff
Index: theme.py
===================================================================
--- theme.py (revision 1343847)
+++ theme.py (working copy)
@@ -98,8 +98,8 @@
}
BOOTSTRAP_CSS_DEFAULTS = (
# ('XPath expression', ['default', 'bootstrap', 'css',
'classes'])
- ("body//table[not(contains(@class, 'table'))]", # TODO: Accurate
?
- ['table', 'table-condensed']),
+ ("body//table[not(contains(concat(' ',normalize-space(@class),'
'),"
+ "' table '))]", ['table', 'table-condensed']),
)
implements(IRequestFilter, ITemplateProvider, ITemplateStreamFilter)
}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:12>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+----------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: reopened
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+----------------------
Changes (by olemis):
* status: closed => reopened
* resolution: fixed =>
Comment:
Replying to [comment:15 gjm]:
> r1345277 simplifies the class adding function but keeps the class order
as before
>
Please revert this changeset . This is what I get using ''python 2.6''
{{{
#!python
>>> from bhtheme import theme
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "bhtheme/theme.py", line 19, in <module>
from collections import OrderedDict
ImportError: cannot import name OrderedDict
>>> import sys
>>> sys.version
'2.6.5 (r265:79063, Apr 16 2010, 13:57:41) \n[GCC 4.4.3]'
}}}
> I think we can close this now. With luck I will forget that this is
worth looking at again.
>
Murphy is everywhere ''':'('''
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:18>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: reopened
Priority: major | Milestone: RC1 for initial release
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-------------------------------------
Changes (by olemis):
* milestone: => RC1 for initial release
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:19>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by gjm):
r1345277 simplifies the class adding function but keeps the class order as
before
I think we can close this now. With luck I will forget that this is worth
looking at again.
#97 is raised to look at the problem with tables in the wiki.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:15>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: closed
Priority: major | Milestone: RC1 for initial release
Component: dashboard | Version:
Resolution: fixed | Keywords:
--------------------------+-------------------------------------
Changes (by gjm):
* status: reopened => closed
* resolution: => fixed
Comment:
With no more discussion, this ticket appears to be complete.
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:21>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
Replying to [comment:11 gjm]:
> OK.. in principle, someone could be using a CSS rule like
>
> {{{
> #!css
> TABLE[class*="foo bar"] {
> width: 50%;
> }
> }}}
>
I was thinking more about the following situation
rule
{{{
#!css
.class2 {
background-color: #FFFFCC;
}
}}}
... and ...
{{{
#!css
.class1 {
background-color: #FF0000;
}
}}}
... and these two tables ...
{{{
#!xml
<table class="class1 class2" >
<!-- Whatever ... -->
</table>
<table class="class2 class1">
<!-- Whatever ... -->
</table>
}}}
[...]
>
> I am still not convinced that we should support this as there is no
particular excuse for requiring a specific order when you can just apply
another class instead. Using {{{TABLE.dirlist}}} style selectors, there is
no order dependency on the classes that I am aware of.
>
If classes are sorted by name then both tables in example above will look
the same , and maybe they should not ... In order to prevent these side-
effects from happening , class order should be preserved ''';)'''
cmiiw anyways ... but I've noticed this before .
> I would be all for simplifying to accept the order decided by the set
type. However, if it is insisted, I would suggest the following as a bit
more readable while there is a lack of a standard ordered set type:
>
> {{{
> #!python
> from collections import OrderedDict
>
> class_list = OrderedDict((k, None) for k in class_list).keys()
> out_classes = ' '.join(class_list)
> }}}
>
+1
> That is just my preference of course but I do want to emphasise that
code readability is very important.
I went to the same school : '''code is for humans''' ... so ...
'''readability matters'''
:)
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:13>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
Replying to [comment:9 gjm]:
> r1343821 provides a mechanism to filter genshi streams to add css
classes
>
> r1343847 adds the conversion for tables
>
cool !
''':)'''
> If I haven't missed anything obvious, I would like to suggest the
following as being a little more obvious about what it is doing.
>
> {{{
> #!python
> def attr_modifier(name, event):
> attrs = event[1][1]
> class_list = attrs.get(name, '').split()
> self.log.debug('BH Theme : Element classes ' + str(class_list))
>
> class_list += classes
> class_list = sorted(set(class_list))
> out_classes = ' '.join(class_list)
> self.log.debug('BH Theme : Inserting class ' + out_classes)
> return out_classes
> return attr_modifier
> }}}
hmmm ... initially I suggest to keep the (''spirit of'' the) following
snippet in the patch (lines 125 , 126 in
[attachment:t88_r1342840_bootstrap_css_filter.diff submitted patch])
{{{
#!python
_classes = ' '.join(c for c in _classes if c in classet and not
classet.discard(c))
}}}
Fact is that the goal is to introduce potentially new classes in the same
order they were before being processed by stream filter . Order matters
when browsers apply CSS to elements in a web page . Hence there's a chance
(i.e. not always but possible ''';)''' for introducing odd behaviors in
case order might be important in a particular situation . Set object is
used in order to remove duplicates . The perfect solution is to use an
ordered set ... but I don't know if there's one such thing anywhere out
there for ''python'' development ... and we can use it for this .
Names and everything else are ok afaict .
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:10>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-----------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: new
Priority: major | Milestone:
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-----------------
Comment (by olemis):
In this second message I'll describe the approach I mentioned [#comment:4
above] . In attached patches you'll find changes related to ideas
mentioned before (e.g. [#comment:4 use stream filters] , [#comment:5 apply
.table class only]) .
In a few words what you see in there is a stream filter in action matching
elements against the same ''XPath'' expression to perform the same task
like before i.e. ensure Bootstrap classes will be there even if not
inserted explicitly in ''Genshi'' templates. In order to add new rules for
default Bootstrap ''CSS'' classes the only thing needed is to add a tuple
of the form `('XPath expression', ['default', 'bootstrap', 'css',
'classes'])` in `BloodhoundTheme.BOOTSTRAP_CSS_DEFAULTS` .
Attached patches implement stream filter approach . They should be applied
against r1342840 in the following order :
{{{
#!sh
$ hg qapplied
t88/t88_r1342840_bootstrap_css_filter.diff
t88/t88_r1342840_bootstrap_css_table.diff
}}}
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:6>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker
Re: [Apache Bloodhound] #88: Generic conversions of tables to
bootstrap
Posted by Apache Bloodhound <bl...@incubator.apache.org>.
#88: Generic conversions of tables to bootstrap
--------------------------+-------------------------------------
Reporter: gjm | Owner: gjm
Type: enhancement | Status: reopened
Priority: major | Milestone: RC1 for initial release
Component: dashboard | Version:
Resolution: | Keywords:
--------------------------+-------------------------------------
Comment (by gjm):
Ah, rats.
OK, for now I am going for the simple beauty of
{{{
#!python
out_classes = ' '.join(set(class_list + classes))
}}}
r1345535 fixes the import error
--
Ticket URL: <https://issues.apache.org/bloodhound/ticket/88#comment:20>
Apache Bloodhound <https://issues.apache.org/bloodhound/>
The Apache Bloodhound (incubating) issue tracker