On 12-09-25 3:59 AM, Nick White wrote:
Sorry, I haven't been able to follow the
discussion 'til now.
I'm glad the skin met with (general) approval, and was happy to read
the fruitful discussion.
One miscellaneous point that's worth making. At present the CSS is
structured so that erudite.css is an exact copy of the original
wordpress CSS, and wikistyle.css has various definitions and
overrides of that to work well as a mediawiki skin. It would
probably make sense to just merge the two files, so things were
clearer and cleaner. There are also quite a few parts of erudite.css
which aren't used at all with the mediawiki theme, which should
probably be removed.
I'll reply to various messages in the thread below.
On Fri, 21 Sep 2012 08:07:48 -0700, Daniel Friesen wrote:
There are a few issues. Some small ones inside
the code and the
file layout seems to have been misunderstood.
Thanks for taking a look. It would
be great if you could enumerate
what these issues were. If you were just planning to fix them
yourself, I can just look at the git log, though.
Most of them are just easier for
me to fix. I've done a few of them so far.
Ok, I'm done.
I've dealt with all the issues I could see except for two.
Firstly, the top navigation. Ideally should really use makeListItem to
generate the links but I don't remember if I included functionality to
let you key in extra patterns for classes/ids.
But more importantly `$this->data['sidebar']['navigation']` is not a
proper way to build a flat header navigation. Though right now we don't
have a standard way to do it in core. So we end up adding a new
navigation message for every skin like this and rewriting the same kind
of code over and over.
Secondly, the new pages list. Doing a DB query like this on every page
view isn't acceptable. Something like this should be cached.
Additionally str_replace on rc_title is not a proper way to output the
text of a new page. The Title class should be used. The way it's being
done will do things like omit the namespace. It's also not properly
escaped so it's a potential XSS vector. Using skin as the Linker is
deprecated. And it's using the wrong linker method to build the link
(though makeListItem might be the right one instead of the Linker).
I do understand that some skins would want to incorporate something like
that. So I would be in favour of building a stripped down NewPagesPager
that doesn't have all the options, caches the result in the wiki's
cache, and then calling it something like NewPagesWidget and making some
others like it for skins that need that kind of functionality.
I'm very
happy to help fix stuff up as appropriate; let me know of
anything that'd be useful.
On Fri, 21 Sep 2012 08:07:48 -0700, Daniel Friesen wrote:
This is really a WP theme shoehorned into MW
That's where it comes from, yes. But to me it doesn't feel
particularly "second class"; the wiki controls aren't hidden away,
and nor are the standard wiki links.
On Fri, 21 Sep 2012 15:34:07 -0400, Sumana Harihareswara wrote:
Nick, one more thing -- have you already tested
your skin on
various mobile devices?
No, that's one thing I haven't done at all. I
don't have access to
any mobile devices, either, so I'd ask others who do to maybe have a
look at it with them, and report any issues with it.
On Mon, 24 Sep 2012 12:56:09 -0700, Daniel Friesen wrote:
The repo has been created and the tarball has
been imported into
it:
https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/skins/erudite.git;a=summa…
Sometime soon I'll try to fix some of the big issues with it.
Cool, thanks for
adding it to that git repo. Let me know if there's
anything I can help with (if I did want to make changes, what would
be the appropriate workflow? a patch to this mailing list?)
Sign up for developer
access:
https://www.mediawiki.org/wiki/Developer_access
You (and everyone else) will be able to submit changes to the repo using
Gerrit.
On Fri, 21 Sep 2012 09:51:36 -0700, Amir E.
Aharoni wrote:
3. Putting the search box at the bottom is a bad
practice in some
WordPress skins, and it shouldn't be carried over.
The search box certainly
could be moved to float right of the main
navigation at the top. To me it looks a little clunkier there, it
certainly makes it easier to find.
On Fri, 21 Sep 2012 09:51:36 -0700, Amir E. Aharoni wrote:
4. The :first-letter CSS selector is not
guaranteed to work nicely
with all languages. This is not, by itself, a reason not to use it;
just take it into the consideration. For example, you can make it
easy to disable it for some languages.
I'm not sure about the :first-letter
stuff in the skin. It looks
nice for english, but characters with diacritics can touch the edge
of the background image, which can make things unclear. I kept it
mainly to stay truer to the wordpress theme this is based on, but
maybe it would be a better idea just to get rid of it.
Right, I think that answers all the outstanding questions people
had. Let me know if not. As I say, I'm happy to help with the skin
however I may, and thanks for taking an interest in it.
Nick
~Daniel Friesen (Dantman, Nadir-Seen-Fire) [
http://daniel.friesen.name]