Item12192: PatternSkin fixes and enhancements for Foswiki 1.2.

This bug item was triggered by changes in Item12178.

---

Changes:

• some re-arrangements after we have removed borders from form steps
• unified button styles
• a couple of small color updates (help and notification blocks, buttons)

-- ArthurClemens - 25 Oct 2012

Yea, I was planning to "improve typography" of PatternSkin. But I won't given the recent debate.

Btw you broke:

1. buttons in NatEditPlugin See below
2. jqButtonSimple aren't plain anymore - DONE nope. see screenshots and error list below
3. all jqButtons have a bold font weight now ... looks odd, i.e. on safari - DONE
4. foswikiRight now has got a display attribute which overrides any display:hidden by other classes on the same element Where is this? Not in my checkins. see screenshots
5. all buttons have display:inline-block property now, and the float attribute is being removed. this not supported by IE7 anymore. Twitter Bootstrap has a fallback, and I tested that it indeed works see discussion below
6. the first child elements have lost their top margin ... looks odd in lots of places. DONE. Changed to only address the headers. nope. see screenshots below

-- MichaelDaum - 26 Oct 2012

I've included your improvement of formsteps, and I have explained my reluctance in implementing. This task is proof of my effort in collaboration.

-- ArthurClemens - 26 Oct 2012

And why the heck are you calling your changes: Fixes and enhancements after "improve typography" changes even though your massive changes have nothing to do with my previous checkins in most of the cases, i.e. wrt buttons? This is totally annoying given the release timeline.

-- MichaelDaum - 26 Oct 2012

I understand you are upset. And I agree that the topic title could be seen as blaming - which was not intended.

I will dive into the newly introduced bugs.

-- ArthurClemens - 26 Oct 2012

Note: change Helvetica to Arial. Done

-- ArthurClemens - 26 Oct 2012

Buttons in nat edit plugin are broken because of styles in NatEditPlugin/default/styles.css. It looks like it implements its own variant of jqButton, which is not necessary because the links also have class jqButton. If I comment out these lines:

/*
.natEditBottomBar a:hover,
.natEditBottomBar a {
  background-color:inherit;
  background-image:url(btn_right.gif);
  background-position:top right;
  background-repeat:no-repeat;
  text-decoration:none;
  cursor:pointer;
  display:block;
  float:left;
  line-height:23px;
  padding:0px;
  margin:0px 8px 0px 0px;
}
.natEditBottomBar li span {
  background-image:url(btn_left.gif);
  background-position:top left;
  background-repeat:no-repeat;
  cursor:pointer;
  display:block;
  float:left;
  line-height:23px;
  margin:0px 3px 0px 0px;
  padding:0px 8px;
}
.natEditBottomBar a:active {
  background-position:bottom right;
}
.natEditBottomBar li a:active span {
  background-position:bottom left;
}
*/

and add

.natEditBottomBar .jqButton {
    margin:0px 5px 0px 0px;
}

the buttons look as expected, and also the hiding works again.

---

About defining a.jqButton instead of just .jqButton - this happens in jquery.button.css as well, and without it, the buttons get blue text.

---

I haven't found the issue about foswikiRight. It would be helpful if you could indicate where this happens.

-- ArthurClemens - 26 Oct 2012

Tested in IE7: the display:inline-block hack works, so floats are not necessary anymore.

-- ArthurClemens - 27 Oct 2012

There still seems to be a more generic problem: jqButtons inside a twisty are still broken. See http://trunk.foswiki.org/Sandbox/WebHome

-- MichaelDaum - 28 Oct 2012

Tested on IE7, IE8, IE9, Chrome, Safari on Mountain Lion. They all look fine. What is wrong?

-- ArthurClemens - 28 Oct 2012

This is how the twisty button looks like using Chrome. Clearing the browser cache did not help. Firefox is okay.

Turns out that this is an ordering problem between jquery.button.css and pattern's styles: they rearrange randomly. So there's a missing dependency.

-- MichaelDaum - 28 Oct 2012

This is what I see:

• Screenshot button Chrome 21 on Mac OSX Mountain Lion:
  

• Screenshot button Chrome 21 on Windows 7:
  

• Screenshot button IE 10 on Windows 8:
  

For testing I use http://www.browserstack.com/.

Please provide some more details on Chrome version and OS.

-- ArthurClemens - 28 Oct 2012

Not a browser problem. The issue is that there are two competing rules for .jqButton span, one adding a background image, the other removing it. The one coming first in linear order has got less precedence.

There are three ways to resolve this: either add a dependency to any ADDTOZONE requiring JQUERYPLUGIN::BUTTON. This will properly arrange the different css files in the html header. The other way to resolve is to move the new button css to PatternSkin's own jquery-ui theme. The theme files will always be arranged properly no matter what. Third way: prefix the PatternSkin specific rules with a patternXXX class, e.g. #patternScreen .jqButton span. That way you don't have to worry about linear ordering and dependencies. However sub-skins of PatternSkin will have a harder time.

-- MichaelDaum - 28 Oct 2012

I will try the second option first. Still I wonder why I don't see the background image from the span, that you've screenshotted.

-- ArthurClemens - 28 Oct 2012

That's probably best as you will also be able to align the other jquery-ui look and feel to your new button style.

-- MichaelDaum - 28 Oct 2012

jqButton styles are now in jquery-ui. Now we need to settle the handling of buttons on the nat edit screen (see above).

-- ArthurClemens - 28 Oct 2012

Yes, the local jqButton version in NatEditPlugin should go away. I'll also deprecate NATEDIT_THEME now that we've got jquery-ui themes.

There are still some issues I see with the latest PatternSkin changes:

• .jqButtonSimple is still decorated and not flat (no gradients etc) -- It is undecorated: check http://trunk.foswiki.org/Sandbox/FoswikiFormStepsTest
• *:first-child removing any top margin is by far too strong. it especially is a pita when using a -1px to balance an extra 1px top width coming with a border. The -1px top margin trick is not working in those cases the element is some first child. so please consider restricting :first-child with more caution by restricting its scope much more. -- I did change this: only headers now have a first-child rule; remains .foswikiFormStep:last-child (the last form step) and .foswikiFormStep p:last-child (the last p in a form step).
• unrelated to any button change, you've added a display:inline to the .foswikiRight right class (not there in the .foswikiLeft class). this overrules any display property added to an element by a sister class assigned to the element. There's really no reason to have any display property on floating elements. Btw. the position:relative on .foswikiLeft and .foswikiRight has got some strange behavior in some cases with regards to adjacent elements and the float not being in vertical rhythm due to the position property for some reason. Removing the position:relative property fixed this. This is unrelated to my changes; it has been there for ages (got imported from tmwiki). But it is wrong indeed. Somehow you have worked around this for longer.

• the display:inline-block added to jqButton still disturbs those cases where a sister class assigned to the element would add a competing display property. Please show an example so I can see if this can be addressed. Please install TopicInteractionPlugin and prepend topicinteraction to the skin path. Then go to http://.../bin/attach/Sandbox/AttachTest?skin=topicinteraction,pattern on your local install. That's where the screenshots below are taken.
• .jqButton not floating to the left looks strange on the upload dialog of TopicInteractionPlugin ... the normal and the pattern theme floating and display behavior being so very different will be a problem for any other form to look okay across themes. We need to harmonize this again. In Base Skin I tackle this in a container (button group) that has a left float. Point is that either we make all buttons float again, or the css styles in jquery.button.css need to use the same floating/displaying behavior as in PatternSkin. Otherwise we will always have somebody craft markup only pleasing one of the button layout flavors. It is major important to harmonize these styles across all skins and themes. Normally people styling buttons shouldn't go into these properties and only change colors, gradients, borders etc, but not change those properties that influence the element's layout itself so fundamentally as you did.

I'll attach a screenshot illustrating the problems.

Errors due to latest pattern skin changes:

Should look like this:

-- MichaelDaum - 29 Oct 2012

Strange I had to tripple check to get your new pattern jquery-ui theme updates. Added to those bug reports that I see fixed now.

Alas I've found some more errors:

• in NatEdit buttons lost half of their decoration on trunk.foswiki.org.
• on my local install they only loose part of the decoration when hovering over the button; that's because hovered links get a background-image:none for no apparent reason.
• .jqButtonSimple has lost its margin in your last checkin. so they are too close to each other now. the margin:0 property should be removed imho.
• can we please make the edit dialog use less white space around the text area again?
• all in all I find the new button design you've created too pale: it barely has contrast in its gradient with a gray font for its label. some laptop screens might not get it.

This change fixes the layout and hiding problems with your new buttons again:

===================================================================
--- jquery-ui.uncompressed.css  (revision 15768)
+++ jquery-ui.uncompressed.css  (working copy)
@@ -1365,7 +1365,6 @@
 */
 .jqButton {
   height: auto;
-  float: none;
   margin: 0;
   background-image: none;
 }
@@ -1379,11 +1378,6 @@
 }
 .jqButton,
 a.jqButton {
-  display: inline-block;
-  *display: inline;
-  /* IE7 inline-block hack */
-
-  *zoom: 1;
   padding: 2px 7px 2px 7px;
   margin-bottom: 0;
   line-height: 22px;

-- MichaelDaum - 29 Oct 2012

It would be useful if jqButton and foswikiButton are similar in use, and if it would be possible to put a button next to a form field without using float for both. You understand I favor inline-block if possible.

Note that the mis-alignment of the close button in TopicInteractionPlugin is caused by the new style of foswikiFormSteps, so unrelated to the button styles.

Also note that the topic actions at the bottom get too much padding when having topicinteraction in the skin path:

-- ArthurClemens - 29 Oct 2012

Hm, not over here. sigh.

Long term those bottom topic actions should go away in favor of having all topic actions in the upper right corner, some of which in a drop down menu.

-- MichaelDaum - 29 Oct 2012

I totally agree. This is what I am working on:

Base Skin dropdown in progress:

The whitespace at the left is room for icons.

-- ArthurClemens - 29 Oct 2012

Some remarks on your menu:

• There should be only one "Edit" action. In the future we will have multiple editors, not only wysiwyg, and wiki text (like ace, code mirror, markitup, whatever). This should be configurable in the topic's preferences.
  • Not sure what you mean. Do away with "raw edit" as menu item?
  • "raw edit" = "Edit wiki text"
  • So if there are multiple raw editors at the same time, they should be called by name?
  • User should just click on an "Edit" link to start editing (no choice hassles on the UI). Which editor kicks in is depending on preference settings fetched from user/web/topic/session/url context as usual. It thus would supersede the NOWYSIWYG binary flag. Edit templates should be able to enforce a specific editor as well. Thats because there is special content that is best edited in a code-editor like ace or code mirror, while content authors do best using wysiwyg, NatEdit as a good choice for power users covering a middle ground.
• The "Edit" action should be a one click direct action, not inside the drop down menu. Otherwise users will have to hover, wait for the popup, slide down, click on "Edit" or "Edit wiki text", versus just one click on an "Edit" button. Same holds for "Attach" and "New topic". These three are so important that they should not be buried inside a drop down.
  • The Edit button itself is clickable. The topmost "Edit" is a duplicate and should probably go. This is not a dropdown button but a fold out menu - the action is on hover (but tap on tablets).
  • That's not obvious. Needs testing.
• A better wording for "Move to trash" is "Delete". Users don't have to be bothered with any technical difference between those two concepts. We don't have a physical "Delete" in Foswiki anyway (one that can't be restored from a Trash). When you add a trash bin icon next to it, the semantics of "Delete" are even more clear.
  • Except that users may not expect to be able to restore. Delete sounds permanent. Mac users have used "Move to Trash" since the beginning.
  • "Delete topic" is simpler to understand than "Move to Trash". The latter needs an understanding what (a) a Trash is and (b) that I can delete stuff by moving it into the trash. Why not call it "Delete" in the first place.
  • Gmail: Delete. Google Docs: Remove. Dropbox: Delete. Evernote: Delete. OK then.
• Setting the topic's parent is part of NatEdit. There's no need for a separate menu entry. Topic parenting is a very marginal feature, imho.
  • Except when there's no NatEdit. We can use a conditional NatSkinPluginEnabled. Better would be a (new) capability check.
  • NatEdit is default on trunk, even for PatternSkin. It has got nothing to do with NatSkin anymore. See this screenshot: .
  • I understand, but this is base skin, not pattern skin.
  • It should use NatEdit as well.
  • I understand the endorsement, but skin authors must be able to choose their edit template.
  • NatEdit is not only an editor: it is a framework for edit templates where skin authors can add, remove and rearrange tabs and things. We really need to provide skin and plugin authors a common ground to craft edit templates that work on a lot of circumstances. Please consider covering NatEdit in your new skin. It is the default from 1.2.0 on.
• Rename and Move are the same, no matter what the target is. So "Rename topic" and "Move to different Web" can be merged into a "Rename or move" menu item and users will decide on the target on the following dialog.
  • Technically they are the same, but psychologically they are different. Still, to save a menu item they can be combined into one.

-- MichaelDaum - 29 Oct 2012

Here's a button test to compare your new pattern buttons with the default ones: http://trunk.foswiki.org/Sandbox/ButtonTest

Some observations:

• when using display:inline-block horizontal spacing between buttons becomes white-space sensitive. white space between buttons is not changing spacings between buttons using float
• %BUTTON{....class="foswikiHidden"}% does not hide the button, nor does any other class other than foswikiHidden bringing in a display:none property; it is overruled by pattern's button style property using display:inline-block. I'll keep experimenting with display:inline-block in jquery.button.css directly, that is one level below the precedence stack.
• the new pattern skin buttons are quite high: compared to an input field next to it
• the foswiki buttons are more crispy and might display better on low end screen displays, imho.
• display error in IE7+8:
  

-- MichaelDaum - 30 Oct 2012

We still have the bug that the jqButton with class foswikiHidden is displayed. Easy to fix though.

-- ArthurClemens - 01 Nov 2012

Yes: remove the display:inline-block properties from pattern theme. They are part of the base css now since I switched JQueryPlugin's buttons to inline-blocks some checkins ago.

-- MichaelDaum - 01 Nov 2012

Done.

-- ArthurClemens - 01 Nov 2012

Found another error:

* {
-webkit-box-sizing: border-box;
-moz-box-sizing: border-box;
box-sizing: border-box;
}

does create some weird spacing in some places:

compare

http://trunk.foswiki.org/bin/edit/Sandbox/FoswikiFormStepsTest?t=1351778680;skin=natedit,plain

to

http://trunk.foswiki.org/bin/edit/Sandbox/FoswikiFormStepsTest?t=1351778681

The tabs at the top have an extra pixel white space.

I find the white space at the top of the edit screen too much.

The line-height in textareas is a bit too high: font-size:12px, line-height:20px. A resulting 18px (using line-height:1.5) would be better.

-- MichaelDaum - 01 Nov 2012

The pattern theme not being used in edit mode is fixed as part of Item12214.

• Great, I was pulling my hairs out. -- ArthurClemens - 01 Nov 2012

-- MichaelDaum - 01 Nov 2012

Found another error:

ui-buttons don't have an icon. That's because you did not copy ui-icons-famfamfam.png to your pattern theme as well.

See http://trunk.foswiki.org/Sandbox/JQueryUIThemeTest

They are there in http://trunk.foswiki.org/Sandbox/JQueryUIThemeTest?theme=foswiki

It also demos the jquery.tabpane error.

-- MichaelDaum - 01 Nov 2012

The gap below the tabs is created because the list items have height of 29px and the containing links are 25px.

They are given 4px less because of the link top padding. But with box-sizing: border-box the height is exactly what you set - no padding calculation needed.

This is generally an easier way to deal with sizes. It is supported in IE8 onwards. See also http://paulirish.com/2012/box-sizing-border-box-ftw/

-- ArthurClemens - 01 Nov 2012

Changing the box model needs intensive testing. Point is: we need to harmonize these fundamental layout properties across all of the project, not only in your jquery-ui pattern skin theme.

A logical consequence is to deprecate use of IE6 and IE7 on foswiki ... which needs a discussion in the scope of a feature request.

Put the other way around: as long as we did not decide on dropping support for IE7, we can't default to the box model you use in pattern skin yet.

Or: how does IE7 behave on a page crafted for a border-box model? I'd expect it to ignore it and do its own legacy box model layout ... which might break things in unexpected ways, as we start seeing now in the case of the tabpanes.

Or: Does it make sense to globally switch to the new box model and still work around IE7's layout differences diverging even more than normal?

Note, the current way to work around the add-padding-to-grid problem discussed by Paul has been mitigated by the use of "contents" divs within layout elements similar to this skelleton:

<div class="main">
  <div class="main-contents">
  </div>
</div>

<style>
.main {
  width:50%;
}
.main-contents {
  padding:10px;
}
</style>

Any width properties are assigned to .main; padding goes to the inner .main-contents class; no padding is ever added to .main.

This of course is more complicated markup than required in the new box model. Yet I see it in use by PatternSkin and I've adopted it in NatSkin. It is compatible across all browsers and all box models.

I do see the benefits of changing the box model, even to simplify the skins' markup significantly. Yet there are some consequences that have to be considered for all of the rest of the project.

This clearly is out of scope for 1.1.6 and 1.2.0.

For now, we have an error in tabpanes being rendered with pattern's jquery-ui theme: there are a few pixels off.

That's why I'd suggest to:

• (a) not yet make use of the new box model and
• (b) fix the height's of tabs as you mentioned above
• (c) continue to experiment with the new box model after 1.1.6 and 1.2.0 are out

-- MichaelDaum - 02 Nov 2012

After some investigation on the tab heights, I can't see how to resolve this on both box models: the current tab's height either adds it's top padding to the height (old model) or does not (new model). In the latter case the tab is not high enough to look attached the pane content. I can't even add a pattern prefix to some specific workarounds as old and new pattern skins have different box models.

-- MichaelDaum - 02 Nov 2012

I haven't covered the tab panes myself completely yet, but will spend some time this weekend.

-- ArthurClemens - 02 Nov 2012

The fix within pattern is simple: just add the top and bottom padding numbers to the height, that is it has got a top padding of 4 and a height of 24, makes the requried 28 in old model; up the height of the .current tab to 28 and the pixel gap is gone.

But please also consider my more general remarks above on changing the box model. So that we don't need skin specific css in plugins. Otherwise you'll end up having to override each and everything in the skin's css, which basically is what you are doing now. Better would be to harmonize the basics related to layout, floating, box model, etc, so that you don't have to on that level of css.

-- MichaelDaum - 02 Nov 2012

I agree more harmonization is needed, which should lead to less overrides in skins.

BTW I noticed on the jquery-ui test page that you provided that the jqTabs button labels are smaller than the jquery-ui ones. Is it caused by an override?

-- ArthurClemens - 02 Nov 2012

No, that's because the jqTabs are locked to 12px font-size, while the jquery-ui ones are 14px inherited by the global font-size settings.

Btw, superfish menus break with box-sizing: border-box.

-- MichaelDaum - 02 Nov 2012

I am going to revert the changes in release branch. Too much has changed for a patch release.

-- ArthurClemens - 02 Nov 2012

Until no obvious glitch shows up, I think we can close this task.

-- MichaelDaum - 15 Mar 2014

If you find this topic by searching for button presentation problems like this one (discussed above):

You may discover (like I did) that this issue is fixed in 2.0.0 and that 2.0.0's PatternSkin is not compatible with 1.1.9.

Fortunately, the fix is simple: Make the JQUERYPLUGIN::THEME css dependent on the JQUERYPLUGIN::BUTTON css.

https://github.com/foswiki/distro/blob/Release01x01/JQueryPlugin/lib/Foswiki/Plugins/JQueryPlugin/Plugins.pm#L150 shows the place where the change must be made. Add JQUERYPLUGIN::BUTTON to the list of dependencies.

The resulting code looks like this:

    Foswiki::Func::addToZone( "head", "JQUERYPLUGIN::THEME",
        <<HERE, "JQUERYPLUGIN::FOSWIKI, JQUERYPLUGIN::UI, JQUERYPLUGIN::BUTTON" );
<link rel="stylesheet" href="$url" type="text/css" media="all" />
HERE

-- MichaelTempest - 07 July 2015

Commenting is disabled while not logged in