Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Wikipedia: Wikitech

Re: [MediaWiki-CVS] SVN: [38677] trunk/phase3

 

 

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded


Simetrical+wikilist at gmail

Aug 6, 2008, 7:59 AM

Post #1 of 4 (366 views)
Permalink
Re: [MediaWiki-CVS] SVN: [38677] trunk/phase3

On Tue, Aug 5, 2008 at 10:18 PM, <dantman[at]svn.wikimedia.org> wrote:
> Revert r38675:
> This commit was clearly not thought out and poorly implemented.
> * The Sanitizer has not been used
> * Proper implementation of this would follow the same convention as the other classNames and have a 'skin-' prefix
> Consistency in the code organization wasn't even kept, a bit of code was just lazily tacked onto the end of another line.

The Sanitizer doesn't need to be used, since any valid PHP identifier
is a valid CSS class. The point about the prefix is valid. Also, we
need to carefully think about how many new classes we want to spam
onto the body element. What's the use-case here? CSS is loaded
conditionally based on skin already, and for JavaScript I'm pretty
sure it's already provided in a variable.

_______________________________________________
Wikitech-l mailing list
Wikitech-l[at]lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


innocentkiller at gmail

Aug 6, 2008, 8:28 AM

Post #2 of 4 (329 views)
Permalink
Re: [MediaWiki-CVS] SVN: [38677] trunk/phase3 [In reply to]

On Wed, Aug 6, 2008 at 10:59 AM, Simetrical
<Simetrical+wikilist[at]gmail.com> wrote:
> On Tue, Aug 5, 2008 at 10:18 PM, <dantman[at]svn.wikimedia.org> wrote:
>> Revert r38675:
>> This commit was clearly not thought out and poorly implemented.
>> * The Sanitizer has not been used
>> * Proper implementation of this would follow the same convention as the other classNames and have a 'skin-' prefix
>> Consistency in the code organization wasn't even kept, a bit of code was just lazily tacked onto the end of another line.
>
> The Sanitizer doesn't need to be used, since any valid PHP identifier
> is a valid CSS class. The point about the prefix is valid. Also, we
> need to carefully think about how many new classes we want to spam
> onto the body element. What's the use-case here? CSS is loaded
> conditionally based on skin already, and for JavaScript I'm pretty
> sure it's already provided in a variable.
>
> _______________________________________________
> Wikitech-l mailing list
> Wikitech-l[at]lists.wikimedia.org
> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>

It is, and I made a comment to that effect on the bug (15052,
for those not following). A "skin" variable was added to the list
of available Javascript variables for just this reason: for times
in which a user script (in this case, widget) wants to do
something conditionally based on skins. If the widget maker is
upset because they have to do their skinning based on a
Javascript variable as opposed to using a class, well then
I'm sorry.

To further Simetrical's point, I would like to see a situation in
which this would prove useful and the skin variable not suffice
before lazily tacking on more classes to the body element
(there's enough as-is, IMO). There's a few other bugs on this
same type of question (classes for subpages, etc.), but I can't
seem to dig up the numbers (someone mentioned them in
#mediawiki last night, but my logs for that are oddly missing...)

-Chad

_______________________________________________
Wikitech-l mailing list
Wikitech-l[at]lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


dan_the_man at telus

Aug 6, 2008, 11:04 AM

Post #3 of 4 (330 views)
Permalink
Re: [MediaWiki-CVS] SVN: [38677] trunk/phase3 [In reply to]

Splarka made a note about the [[MediaWiki:Print.css]] and
[[User:Username/print.css]] proposals.
A skin-skinname class would be useful to prevent the need for multiple
of those, one for each skin.

I also note that it's useful for code that does belong in Common.css,
but needs minor skin tweaks. That's actually especially useful if there
are two or three skins that use similar locations or styles for that
common code.
Say css changes for message boxes inside of the content area, on skins
that have a dark content area. You could put it in Skinname.css, but
then you end up duplicating the code for each skin needing it,
separating the code from the rest of the styling for those boxes, and
also increasing the chances you'll forget to update the code for one of
those skins.

If you read the bugzilla entry you'll notice another issue with the
commit. I didn't notice it till after, but all that was added was a lazy
get_class( $this ); which is an unreliable way to get the name of the
skin, we have member variables for that. I have doubts about if the code
actually worked right.

~Daniel Friesen(Dantman, Nadir-Seen-Fire) of:
-The Nadir-Point Group (http://nadir-point.com)
--It's Wiki-Tools subgroup (http://wiki-tools.com)
--The ElectronicMe project (http://electronic-me.org)
--Games-G.P.S. (http://ggps.org)
-And Wikia ACG on Wikia.com (http://wikia.com/wiki/Wikia_ACG)
--Animepedia (http://anime.wikia.com)
--Narutopedia (http://naruto.wikia.com)

Chad wrote:
> On Wed, Aug 6, 2008 at 10:59 AM, Simetrical
> <Simetrical+wikilist[at]gmail.com> wrote:
>
>> On Tue, Aug 5, 2008 at 10:18 PM, <dantman[at]svn.wikimedia.org> wrote:
>>
>>> Revert r38675:
>>> This commit was clearly not thought out and poorly implemented.
>>> * The Sanitizer has not been used
>>> * Proper implementation of this would follow the same convention as the other classNames and have a 'skin-' prefix
>>> Consistency in the code organization wasn't even kept, a bit of code was just lazily tacked onto the end of another line.
>>>
>> The Sanitizer doesn't need to be used, since any valid PHP identifier
>> is a valid CSS class. The point about the prefix is valid. Also, we
>> need to carefully think about how many new classes we want to spam
>> onto the body element. What's the use-case here? CSS is loaded
>> conditionally based on skin already, and for JavaScript I'm pretty
>> sure it's already provided in a variable.
>>
>> _______________________________________________
>> Wikitech-l mailing list
>> Wikitech-l[at]lists.wikimedia.org
>> https://lists.wikimedia.org/mailman/listinfo/wikitech-l
>>
>>
>
> It is, and I made a comment to that effect on the bug (15052,
> for those not following). A "skin" variable was added to the list
> of available Javascript variables for just this reason: for times
> in which a user script (in this case, widget) wants to do
> something conditionally based on skins. If the widget maker is
> upset because they have to do their skinning based on a
> Javascript variable as opposed to using a class, well then
> I'm sorry.
>
> To further Simetrical's point, I would like to see a situation in
> which this would prove useful and the skin variable not suffice
> before lazily tacking on more classes to the body element
> (there's enough as-is, IMO). There's a few other bugs on this
> same type of question (classes for subpages, etc.), but I can't
> seem to dig up the numbers (someone mentioned them in
> #mediawiki last night, but my logs for that are oddly missing...)
>
> -Chad
>
_______________________________________________
Wikitech-l mailing list
Wikitech-l[at]lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l


Simetrical+wikilist at gmail

Aug 6, 2008, 11:24 AM

Post #4 of 4 (327 views)
Permalink
Re: [MediaWiki-CVS] SVN: [38677] trunk/phase3 [In reply to]

On Wed, Aug 6, 2008 at 2:04 PM, Daniel Friesen <dan_the_man[at]telus.net> wrote:
> Splarka made a note about the [[MediaWiki:Print.css]] and
> [[User:Username/print.css]] proposals.
> A skin-skinname class would be useful to prevent the need for multiple
> of those, one for each skin.

I guess so, yes . . . is that the only one people can think of where
there's no other nice way to do it? If it is, I think that
MediaWiki:Skinname/print.css and MediaWiki:Common/print.css (or
whatever) would be more consistent with how we currently do things.

> I also note that it's useful for code that does belong in Common.css,
> but needs minor skin tweaks. That's actually especially useful if there
> are two or three skins that use similar locations or styles for that
> common code.
> Say css changes for message boxes inside of the content area, on skins
> that have a dark content area. You could put it in Skinname.css, but
> then you end up duplicating the code for each skin needing it,
> separating the code from the rest of the styling for those boxes, and
> also increasing the chances you'll forget to update the code for one of
> those skins.

You should just use cascading here. Put the bulk in Common.css, and
put the actual color values (or whatever needs per-skin customization)
in Skinname.css.

_______________________________________________
Wikitech-l mailing list
Wikitech-l[at]lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Wikipedia wikitech RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact lists@gossamer-threads.com
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.