You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bloodhound.apache.org by Olemis Lang <ol...@gmail.com> on 2013/03/22 02:14:10 UTC

[REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

On 3/15/13, rjollos@apache.org <rj...@apache.org> wrote:
> Author: rjollos
> Date: Fri Mar 15 11:03:43 2013
> New Revision: 1456892
>
[...]
>  .searchable em {
> -    font-style: inherit;
> -    font-weight: bold;
> + font-style: inherit;
> + font-weight: bold;
>  }
>

This change (well ... the original including `font-weight : bold` ;)
is breaking the basic notion of <em> tags being rendered in italics
style . This has some impact on WikiFormatting , as well as other
parts of the GUI .

Please revert to what it was before .

-- 
Regards,

Olemis.

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Olemis Lang <ol...@gmail.com>.
On 3/22/13, Branko Čibej <br...@wandisco.com> wrote:
> On 22.03.2013 10:24, Anze Staric wrote:
>> The change was introduced in r1452190 with match highlighting in
>> search. The tag <em> was chosen as the tag to mark matches based on
>> the default choise of Solr (<em> for emphasis -
>> http://lucene.apache.org/solr/4_1_0/tutorial.html#Highlighting).
>>
>> Css was modified to match the look of the google search results (bold
>> matches). As it only changes style inside elements marked as
>> searchable it was not supposed to break anything outside the search
>> results. I forgot to check whether the css class was used in other
>> contexts. The following patch replaces the name of the class marking
>> the search results that need to be highlighted.
>
> <em>phasis does not imply italic text, so in general it's fine to have
> bold emphasis instead of italic (or underlined, or different font or
> colour, or whatever). Since the definitions is scoped within
> class=highlight_matches, I don't really see a problem.
>

The problem is that the way it was before it was breaking
WikiFormatting styling . Not anymore . Beyond that in other contexts I
have no objection to <em>phasize text in any way we might consider
appropriate as long as it will not introduce regressions like this one
.

... so everything seems to be back to normal now ;)

-- 
Regards,

Olemis.

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Olemis Lang <ol...@gmail.com>.
On 3/22/13, Andrej Golcov <an...@digiverse.si> wrote:
> Agree, the suggested patch provides a good isolation of <em> styling
> for bhsearch results.
> Committed in r1459718.
>

thanks andrej and anze .

-- 
Regards,

Olemis.

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Andrej Golcov <an...@digiverse.si>.
Agree, the suggested patch provides a good isolation of <em> styling
for bhsearch results.
Committed in r1459718.

Cheers, Andrej

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Branko Čibej <br...@wandisco.com>.
On 22.03.2013 10:24, Anze Staric wrote:
> The change was introduced in r1452190 with match highlighting in
> search. The tag <em> was chosen as the tag to mark matches based on
> the default choise of Solr (<em> for emphasis -
> http://lucene.apache.org/solr/4_1_0/tutorial.html#Highlighting).
>
> Css was modified to match the look of the google search results (bold
> matches). As it only changes style inside elements marked as
> searchable it was not supposed to break anything outside the search
> results. I forgot to check whether the css class was used in other
> contexts. The following patch replaces the name of the class marking
> the search results that need to be highlighted.

<em>phasis does not imply italic text, so in general it's fine to have
bold emphasis instead of italic (or underlined, or different font or
colour, or whatever). Since the definitions is scoped within
class=highlight_matches, I don't really see a problem.

-- Brane


-- 
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com


Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Anze Staric <an...@gmail.com>.
The change was introduced in r1452190 with match highlighting in
search. The tag <em> was chosen as the tag to mark matches based on
the default choise of Solr (<em> for emphasis -
http://lucene.apache.org/solr/4_1_0/tutorial.html#Highlighting).

Css was modified to match the look of the google search results (bold
matches). As it only changes style inside elements marked as
searchable it was not supposed to break anything outside the search
results. I forgot to check whether the css class was used in other
contexts. The following patch replaces the name of the class marking
the search results that need to be highlighted.

--- bloodhound_theme/bhtheme/htdocs/bloodhound.css (revision 1459423)
+++ bloodhound_theme/bhtheme/htdocs/bloodhound.css (working copy)
@@ -476,7 +476,7 @@
  text-decoration: underline;
 }

-.searchable em {
+.highlight_matches em {
  font-style: inherit;
  font-weight: bold;
 }
Index: bloodhound_search/bhsearch/templates/bhsearch.html
===================================================================
--- bloodhound_search/bhsearch/templates/bhsearch.html (revision 1459423)
+++ bloodhound_search/bhsearch/templates/bhsearch.html (working copy)
@@ -126,7 +126,7 @@
                       <tr class="${'odd' if idx % 2 else 'even'}
prio${result.priority_value}${
                         ' added' if 'added' in result else ''}${
                         ' changed' if 'changed' in result else ''}${
-                        ' removed' if 'removed' in result else ''} searchable">
+                        ' removed' if 'removed' in result else ''}
highlight_matches">
                         <py:for each="idx, header in
enumerate(headers)" py:choose="">
                           <py:with vars="name = header.name; value =
result[name]; hilited_value=result['hilited_' + name]; title = _('View
')+ result['type']">
                             <td py:when="name == 'id'" class="id"><a
href="$result.href" title="${title}"
@@ -154,8 +154,8 @@
                 <!--Rendering results in free form-->
                 <dl id="results">
                   <py:for each="result in results">
-                    <dt><a href="${result.href}"
class="searchable">${result.title}</a></dt>
-                    <dd class="searchable">${result.hilited_content
or result.content}</dd>
+                    <dt><a href="${result.href}"
class="highlight_matches">${result.title}</a></dt>
+                    <dd
class="highlight_matches">${result.hilited_content or
result.content}</dd>
                     <dd>
                       <py:if test="result.author"><span
class="author" i18n:msg="author">By
${format_author(result.author)}</span> &mdash;</py:if>
                       <span class="date">${result.date}</span>


Anze

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Ryan Ollos <ry...@wandisco.com>.
On Thu, Mar 21, 2013 at 9:42 PM, Olemis Lang <ol...@gmail.com> wrote:

> On 3/21/13, Ryan Ollos <ry...@wandisco.com> wrote:
> > On Thu, Mar 21, 2013 at 6:14 PM, Olemis Lang <ol...@gmail.com> wrote:
> >
> >> On 3/15/13, rjollos@apache.org <rj...@apache.org> wrote:
> >> > Author: rjollos
> >> > Date: Fri Mar 15 11:03:43 2013
> >> > New Revision: 1456892
> >> >
> >> [...]
> >> >  .searchable em {
> >> > -    font-style: inherit;
> >> > -    font-weight: bold;
> >> > + font-style: inherit;
> >> > + font-weight: bold;
> >> >  }
> >> >
> >>
> >> This change (well ... the original including `font-weight : bold` ;)
> >> is breaking the basic notion of <em> tags being rendered in italics
> >> style . This has some impact on WikiFormatting , as well as other
> >> parts of the GUI .
> >>
> >> Please revert to what it was before .
> >
> >
> > That revision should have only changed the indentation to make it the
> same
> > throughout the file. The diff you show is only a change in indentation as
> > far as I can see.
> >
>
> Very obvious . That's why I said «well ... the original including
> `font-weight : bold` ;)» . That one is where my local blame tool led
> me and I saw no point in tracking changes prior to that .
>
> The message is «that should not be bold , no matter where that change
> was applied in first place» .


So basically you have replied to my commit that *did not* introduce the
problem, stated that there was a problem and that the line in question
should be reverted "to what it was before" and told me to fix it even
though you have not yet determined who introduced the change in question or
when this regression may have been was introduced.

If you'd like a change to be made, determine who made the change in
question and raise the issue with that person so that you can have a
productive discussion about the change. Then, work with that person to
produce a patch.

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Olemis Lang <ol...@gmail.com>.
On 3/21/13, Ryan Ollos <ry...@wandisco.com> wrote:
> On Thu, Mar 21, 2013 at 6:14 PM, Olemis Lang <ol...@gmail.com> wrote:
>
>> On 3/15/13, rjollos@apache.org <rj...@apache.org> wrote:
>> > Author: rjollos
>> > Date: Fri Mar 15 11:03:43 2013
>> > New Revision: 1456892
>> >
>> [...]
>> >  .searchable em {
>> > -    font-style: inherit;
>> > -    font-weight: bold;
>> > + font-style: inherit;
>> > + font-weight: bold;
>> >  }
>> >
>>
>> This change (well ... the original including `font-weight : bold` ;)
>> is breaking the basic notion of <em> tags being rendered in italics
>> style . This has some impact on WikiFormatting , as well as other
>> parts of the GUI .
>>
>> Please revert to what it was before .
>
>
> That revision should have only changed the indentation to make it the same
> throughout the file. The diff you show is only a change in indentation as
> far as I can see.
>

Very obvious . That's why I said «well ... the original including
`font-weight : bold` ;)» . That one is where my local blame tool led
me and I saw no point in tracking changes prior to that .

The message is «that should not be bold , no matter where that change
was applied in first place» .

-- 
Regards,

Olemis.

Apache™ Bloodhound contributor
http://issues.apache.org/bloodhound

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [REGRESSION] What should be italics is rendered as bold text WAS: svn commit: r1456892 - /incubator/bloodhound/trunk/bloodhound_theme/bhtheme/htdocs/bloodhound.css

Posted by Ryan Ollos <ry...@wandisco.com>.
On Thu, Mar 21, 2013 at 6:14 PM, Olemis Lang <ol...@gmail.com> wrote:

> On 3/15/13, rjollos@apache.org <rj...@apache.org> wrote:
> > Author: rjollos
> > Date: Fri Mar 15 11:03:43 2013
> > New Revision: 1456892
> >
> [...]
> >  .searchable em {
> > -    font-style: inherit;
> > -    font-weight: bold;
> > + font-style: inherit;
> > + font-weight: bold;
> >  }
> >
>
> This change (well ... the original including `font-weight : bold` ;)
> is breaking the basic notion of <em> tags being rendered in italics
> style . This has some impact on WikiFormatting , as well as other
> parts of the GUI .
>
> Please revert to what it was before .


That revision should have only changed the indentation to make it the same
throughout the file. The diff you show is only a change in indentation as
far as I can see.