Frontend improvements #93

Merged
merged 29 commits into from Jun 30, 2012

Conversation

Projects
None yet
5 participants
Collaborator

selfthinker commented Apr 9, 2012

selfthinker added some commits Apr 8, 2012

removed align attribute from images and inserted them only in the fee…
…d (FS#1351)

can someone with better regex skills please revise this?
load only one stylesheet for all modes instead of three
Instead of three stylesheets for 'all', 'screen' and 'print' modes, they are all loaded into a single stylesheet by wrapping all screen styles in a "@media screen {}" and all print styles in a "@media print {}". The 'all' mode is not wrapped in anything.
Potential issues with existing CSS: If any of your screen or print CSS files already contain any "@media" syntax, the CSS will probably break. In that case please add any CSS with "@media" in it to the 'all' mode instead!

Also, the 'rtl' mode is hereby deprecated. Please just prefix any RTL styles within your normal CSS files with "[dir=rtl]". This also fixes that RTL styles cannot be added for 'all' or 'print' modes.
rewrote and improved HTML for TOC
Attention: Template authors need to adjust their CSS!

Original structure:
div.toc >
  div#toc__header.tocheader.toctoggle > span#toc__toggle.toc_close|toc_open > span
  div#toc__inside > ul.toc > li.level1 > div.li > span.li > a.toc

New structure:
div#dw__toc.open|close >
  h3 > strong > span
  ul.toc > li.toc > div.li > a
improved HTML for search results
Attention: Template authors need to adjust their CSS!

Original structure:
div.search_result >
  a.wikilink1 > span.search_cnt
  br
  div.search_snippet

New structure:
dl.search_results >
  dt > a.wikilink1
  dd
feed.php
+ // add alignment for images
+ $content = preg_replace('/class="medialeft"/s', 'class="medialeft" align="left"', $content);
+ $content = preg_replace('/class="mediaright"/s', 'class="mediaright" align="right"', $content);
+
@splitbrain

splitbrain Apr 15, 2012

Owner

memo to myself: to avoid having this modifying actual content, it should at least match the image tag as well. The tag would be HTML encoded for actual content, so it should be safe to preg_replace it.

Owner

splitbrain commented Apr 15, 2012

AFAIR we said we would phase out support for the "default" template according to our deprecation rules. This means we'll need to have the CSS changed to this new HTML in the default template as well. Correct?

Collaborator

selfthinker commented Apr 15, 2012

Yes, I've made all the changes to both templates (default and dokuwiki).

splitbrain added some commits Apr 20, 2012

made TOC togling script more generic
Instead of a dedicated function to toggle the TOC we now have a function
that allows to use this functionality everywhere. This will be used to
toggle the sidebar in the mobile view (in an upcoming patch).

Note, this required some changes to the CSS (to make it more generic).
The CSS is still located in the TOC sections but should probably be
moved into its own section instead.
Owner

splitbrain commented on 5dcb125 Apr 20, 2012

The styles of the sidebar toggle and the TOC toggle do not match 100% yet. The sidebar font size seems to be smaller. Screenshot: http://i.imgur.com/GePEo.png

Owner

splitbrain commented on c8388e4 Apr 20, 2012

@selfthinker should have a look at my CSS changes and see if any mess needs cleanup

Collaborator

selfthinker replied Apr 22, 2012

I just pushed some improvements.

I don't think the CSS should be moved into its own section, because what about the styles would you define belongs to toggling and what belongs to the TOC? In our case it's more a coincidence that the TOC and sidebar toggling share the same styles. But the functionality of toggling could take any styles... The only thing which might be worth extracting from there are the arrows...?

This is a bit cheeky. ;-)
I don't think leaving it like this makes sense in the long run. Think about multi-lingual setups for example... I think another $lang entry makes sense here.

Collaborator

selfthinker replied Apr 22, 2012

I'm also not sure about using the "a11y" class, as the content is semantically not very relevant...

Collaborator

selfthinker replied Apr 22, 2012

I just removed the "a11y" and set it to "display: none" accordingly.

This means that the sidebar toggle will not appear in tablet mode. Was that your intention?

Owner

splitbrain replied Apr 23, 2012

Hmm. What's tablet mode? I tried the above on my tablet in portrait view and worked fine. In landscape view, the normal site is shown.

Collaborator

selfthinker replied Apr 23, 2012

There are two different style sets, one for up to 768px screen widths (= most tablets and mobile phones) and one for up to 480px screen widths (most mobile phones). And the tablet style is very similar to the normal desktop style, except that the sidebar (and the TOC) is above the content and some paddings are reduced. The site, user and page tools are only removed and replaced with the mobile tools dropdown in mobile mode.

I guess it's not quite working because of the max-device-width: 960px we introduced for new iPhones. There is also a different way we could solve that (with min-device-pixel-ratio: 2). We definitely need to experiment a bit more with those...

selfthinker and others added some commits Apr 22, 2012

match on img tag in feed's img align replacement
this should avoid false positive matches on user content (which is
already escaped at this point)
Contributor

turnermm commented on 3c86b7e May 8, 2012

PHP manual recommends str_replace when regex isn't needed: http://ca.php.net/str_replace

$content = str_replace('class="mediaright"', 'class="mediaright" align="right"', $content);

Contributor

turnermm commented May 8, 2012

See my suggestion on feed.php

@@ -1711,11 +1716,11 @@ function html_TOC($toc){
if(!count($toc)) return '';
global $lang;
$out = '<!-- TOC START -->'.DOKU_LF;
- $out .= '<div class="toc">'.DOKU_LF;
- $out .= '<div class="tocheader toctoggle" id="toc__header">';
@selfthinker

selfthinker Jun 29, 2012

Collaborator

Yup, fair point. I'll do that during the next few days.

@@ -383,11 +378,11 @@ div.dokuwiki img.mediacenter {
}
/* smileys */
-div.dokuwiki img.middle {
+div.dokuwiki img.icon {
@selfthinker

selfthinker Jul 8, 2012

Collaborator

Yes, it does. I don't think the "more descriptive class [to] allow the separate styling" is necessary.

selfthinker added a commit that referenced this pull request Jun 30, 2012

@selfthinker selfthinker merged commit 2982014 into master Jun 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment