Opened 7 years ago

Closed 7 years ago

#15675 closed defect (fixed)

dojox.mobile.Heading back ButtonArrow background-color and image not settable by CSS

Reported by: Bill Reed Owned by: Eric Durocher
Priority: high Milestone: 1.8
Component: DojoX Mobile Version: 1.8.0b1
Keywords: Cc: Adam Peller, cjolif, Damien Mandrioli
Blocked By: Blocking:

Description

For Maqetta.org theme editor we could modify the color and gradient of the heading back button by adding cascade rules to change the selector

.mblArrowButtonHead { position: absolute; top: 11px; left: 5px; width: 20px; height: 19px; border: 1px solid #3A4655; -webkit-transform: scale(0.7, 1) rotate(45deg); background-image: -webkit-gradient(linear, left top, right bottom, from(#8EA4C1), to(#4A6C9B), color-stop(0.5, #5877A2), color-stop(0.5, #476999)); }

In dojo 1.8 beta the CSS rule is no longer used to set the background-image and background-color they are now set on the HTML element

The code that builds the Heading widget seems to be adding the background-image and color to the element at runtime and not using css classes.

element.style { background-color: #5877A2; background-image: -webkit-gradient(linear, 0% 0%, 100% 100%, from(#8EA4C1), color-stop(0.5, #5877A2), color-stop(0.5, #476999), to(#4A6C9B)); }

Attachments (5)

head button cascadeJPG.JPG (184.1 KB) - added by Bill Reed 7 years ago.
heade button casce
ToolBarButton.js.patch (1.1 KB) - added by Bill Reed 7 years ago.
proposed patch
ToolBarButton.js.2.patch (2.2 KB) - added by Bill Reed 7 years ago.
better implementation
ToolBarButton-fix.patch (3.4 KB) - added by ykami 7 years ago.
15675-compatFixes.patch (1.9 KB) - added by cjolif 7 years ago.
Patch for the issue from Kamiyama, Christophe and Adrian (IBM, CCLA)

Download all attachments as: .zip

Change History (30)

comment:1 Changed 7 years ago by Adam Peller

Cc: Adam Peller added

Eric, sorry for the late notice, but if we could have this addressed in time for 1.8, that would be helpful

comment:2 Changed 7 years ago by cjolif

Cc: cjolif added

comment:3 Changed 7 years ago by cjolif

Cc: Damien Mandrioli added

comment:4 Changed 7 years ago by cjolif

This happened as part of [27600]

comment:5 Changed 7 years ago by cjolif

Using mblToolBarButtonArrow instead of mblArrowButtonHead might solve the issue. Indeed the backgroundImage is set explicitly but only when already found on the node initially and then it is modified but not totally erased. Using that new class you should be able to set the one you want and if I'm not mistaken ToolBarButton should re-use it when setting it explicitly.

Last edited 7 years ago by cjolif (previous) (diff)

Changed 7 years ago by Bill Reed

Attachment: head button cascadeJPG.JPG added

heade button casce

comment:6 Changed 7 years ago by Bill Reed

Using mblToolBarButtonArrow has no effect on the arrow head

adding the following rule to my css: .mblToolBarButtonArrow { background-color: #FFA4A4; background-image: -webkit-gradient(linear, 0% 0%, 100% 100%, from(#FFE2E2), to(#A4A4A4));

}

I have attached a image of the Cascade in chrome debuuger http://bugs.dojotoolkit.org/attachment/ticket/15675/head%20button%20cascadeJPG.JPG

comment:7 Changed 7 years ago by cjolif

ok I misread the code actually the backgroundImage is not set when already found on the arrowNode but when already found on the *bodyNode* (the main node of the button not the arrow part). That explains why my suggestion does not work.

So another solution is to put the gradient on your button and it should automatically be adapted to your arrow.

Something like

.mblToolBarButtonBody.mblColorDefault {
  background-color: #FFA4A4;
  background-image: -webkit-gradient(linear, 0% 0%, 0% 100%, from(#FFE2E2), to(#A4A4A4));
}

is that working better?

It seems the mblToolBarButtonArrow is triggered only if no background image is found on the main button node. Which is indeed a bit strange. Maybe the code should be improve by not only looking into the bodyNode but also the arrowNode to check if a pre-existing backgroundImage is here.

comment:8 Changed 7 years ago by Bill Reed

Changing the mblToolBarButtonBody does indeed change the arrow color when the widget is created. The issue is that we allow the user to change the button color in the page designer and theme editor. When the user changes the button color we update the CSSRule in the style sheet so the color change is reflected in the widget on screen.

The way the widget now behaves only the button body reflects the new color and the widget needs to be destroyed and recreated in order to reflect the color change in the head of the button.

Changed 7 years ago by Bill Reed

Attachment: ToolBarButton.js.patch added

proposed patch

comment:9 Changed 7 years ago by Bill Reed

I have done some digging through the dojo code and this is what I have found

dojox.mobile.Heading.js creates a ToolBarButton? for the back button

_setBackAttr: function(/*String*/back){
			// tags:
			//		private
			this._set("back", back);
			if(!this.backButton){
				this.backButton = new ToolBarButton({
					arrow: "left",
					label: back,
					moveTo: this.moveTo,
					back: !this.moveTo,
					href: this.href,
					transition: this.transition,
					transitionDir: -1
				});
				this.backButton.placeAt(this.domNode, "first");
			}else{
				this.backButton.set("label", back);
			}
			this.resize();
		},

ToolBarButton? gets it's background-image using the CSS classs

.mblColorDefault {
color: red;
background-image: -webkit-gradient(linear, left top, left bottom, from(#F6F6F6), to(red));
background-image: -o-linear-gradient(#F6F6F6, red);
background-image: -ms-linear-gradient(#F6F6F6, red);
background-image: -moz-linear-gradient(#F6F6F6, red);
background-image: -webkit-linear-gradient(#F6F6F6, red);
background-image: linear-gradient(#F6F6F6, red);
}

In ToolBarButton?.js buildRendering at about line: 93 we have code that is in a setTimeout that sets the arrow color

setTimeout(function(){ // for programmatic instantiation
				_this._updateArrowColor();
			}, 0);

_updateArrowColor gets the color and image from the button body and uses them to set the Arrow color

_updateArrowColor: function(){
			if(this.arrowNode && !has("ie")){
				domStyle.set(this.arrowNode, "backgroundColor", domStyle.get(this.bodyNode, "backgroundColor"));
				var s = domStyle.get(this.bodyNode, "backgroundImage");
				if(s === "none"){ return false; }					
				domStyle.set(this.arrowNode, "backgroundImage",
							 s.replace(/\(top,/, "(top left,") // webkit new
							 .replace(/0% 0%, 0% 100%/, "0% 0%, 100% 100%") // webkit old
							 .replace(/50% 0%/, "0% 0%")); // moz
			}
			return true;
		},

If I understand the code correctly, the body button color is being set my the CSS class mblColorDefault So if we add that class to the Arrow span and remove the call to _updateArrowColor we would still have code that gets the arrow background from the body.

I have attached a patch file with my proposed fix.

Last edited 7 years ago by cjolif (previous) (diff)

comment:10 Changed 7 years ago by ykami

Unfortunately, I don't think an approach like your patch works. _updateArrowColor() calculates the arrow's gradient color from the body's one so that users, who want to override the default color, do not need to specify two styles, one for the arrow and the other for the body. (Note that the two are not the same.) It is for users' convenience. mblColorPink in test_Heading.html is an example of user-defined custom color. You can see it has only one gradient color definition. (Just ignore the compat definitions.)

If you want to dynamically change the gradient color, I think there are two options.

  1. call _updateArrowColor() after applying a custom color to the button body.
  2. prepare a calculated style for arrow, and apply it to the arrow when you change the body color.

comment:11 Changed 7 years ago by cjolif

It looks like this arrow gradient automatic computation is the only dojox/mobile feature that breaks the ability to dynamically switch the style of a dojox/mobile application. If that is exact, shouldn't we consider the ability to unplug that mechanism for this use-case?

comment:12 Changed 7 years ago by Bill Reed

I just tested the proposed patch with dojox/mobile/tests/test_Heading.html and it works as expected, the button and head have the install color of defaultColor="mblColorBlue" and when the user selects the button the color of the button and head change to selColor="mblColorPink"

But perhaps a better implementation is to use _updateArrowColor to change the class like what is done in _setSelectedAttr I have attached that patch as well.

_updateArrowColor: function(/*Boolean*/selected){
			if(this.arrowNode && !has("ie")){
				if(selected){
					domClass.replace(this.arrowNode, this.selColor, this.defaultColor);
				}else{
					domClass.replace(this.arrowNode, this.defaultColor, this.selColor);
				}
				/*domStyle.set(this.arrowNode, "backgroundColor", domStyle.get(this.bodyNode, "backgroundColor"));
				var s = domStyle.get(this.bodyNode, "backgroundImage");
				if(s === "none"){ return false; }					
				domStyle.set(this.arrowNode, "backgroundImage",
							 s.replace(/\(top,/, "(top left,") // webkit new
							 .replace(/0% 0%, 0% 100%/, "0% 0%, 100% 100%") // webkit old
							 .replace(/50% 0%/, "0% 0%")); // moz
*/			}
			return true;

The issue that we have with your suggestion of call _updateArrowColor() after applying a custom color to the button body. Is that Maqetta is an IDE when the user changes a style in the theme editor it is applied to a CSS style sheet, The user could be editing N number of HTML pages in the IDE that share that theme (CSS style). So when the user changes the color of the button we need to reflect that color change in all the HTML pages that are using that style in the IDE. So we would need to update N number of pages and N number of widgets. We would need to add custom code to support this one widget that behaves diffidently than all the other widgets in the dojo toolkit.

Please consider the second patch I have attached, and keep in mind this is not changing the color on one widget in one page, we need to reflecting that change across all widgets of that type on all the pages that the user is editing that are sharing the theme....

Changed 7 years ago by Bill Reed

Attachment: ToolBarButton.js.2.patch added

better implementation

comment:13 Changed 7 years ago by Damien Mandrioli

The second patch breaks the scenario where you style toolbar buttons using CSS and the mblToolBarButtonBody class:

	/* Customize toolbar buttons background */
        .mblToolBarButtonBody{
		background-image: XXX
		background-image: -webkit-gradient(XXX)
		background-color: XXX;
	}

If you apply the patch, this style block is not applied to the arrow. This can break existing applications.

Last edited 7 years ago by Damien Mandrioli (previous) (diff)

comment:14 in reply to:  13 Changed 7 years ago by Bill Reed

Replying to dmandrioli:

The second patch breaks the scenario where you style toolbar buttons using CSS and the mblToolBarButtonBody class:

	/* Customize toolbar buttons background */
        .mblToolBarButtonBody{
		background-image: XXX
		background-image: -webkit-gradient(XXX)
		background-color: XXX;
	}

If you apply the patch, this style block is not applied to the arrow. This can break existing applications.

We are already breaking existing application by changing the head button implementation to toolBarButton. We have many users of Maqetta that have been creating mobile UI with Maqetta and Dojo 1.7 some of them have been modifying the default themes provided by dojo 1.7. With customized mobile themed application written with dojo 1.7. Using CSS classes to set the back button head will be broken with dojo 1.8. So no matter which way we go some application will be broken.

The dojo 1.8 code has problems as it is, the setTimeout is problematic. If the connection is slow and the CSS style sheet has not been loaded yet by the browser the setTimeout pop's before the background color is set on the body and the head is not colored to match the style. I have encountered this bug.

Why would the user expect a class called mblToolBarButtonBody would effect the mblToolBarButtonArrow ? I think it is logical that changes to the class mblToolBarButtonBody would not effect the Arrow heads, I would argue that it is a bug to have changes madeto mblToolBarButtonBody also change the Arrow head.

comment:15 Changed 7 years ago by Bill Reed

Note that the ToolBarButton? gets it's border color from

.mblToolBarButtonBody {
display: inline-block;
position: relative;
overflow: hidden;
border-radius: 0;
border: 1px solid #9B9B9B;
border-bottom-color: #767676;
text-shadow: rgba(255, 255, 255, 0.5) 0 1px 0;
}

And the ToolBarButtonArrow? gets it border color from

.mblToolBarButtonArrow {
position: absolute;
top: 5px;
width: 20px;
height: 19px;
border-radius: 1px;
-webkit-transform: scale(0.7, 1.05) rotate(45deg);
border: 1px solid #9B9B9B;
border-right-color: #767676;
border-bottom-color: #767676;
}

So for the user to change the borer color they must update both classes with the new border-color.

comment:16 Changed 7 years ago by ykami

I just tested the proposed patch with dojox/mobile/tests/test_Heading.html and it works as expected

I think your arrow has a gradient color which is not rotated correctly, doesn't it?
If it worked, _updateArrowColor() would not be necessary from the beginning.
I think the problem is that _updateArrowColor() is applying the styles directly to the node. That prevents you from overriding the styles.

How about adding css class definition instead of applying styles to the node like the patch I attach?

If the connection is slow and the CSS style sheet has not been loaded yet by the browser the setTimeout pop's before the background color is set on the body and the head is not colored to match the style.

That setTimeout is not for waiting for css to load. It was necessary to read the applied style values. If you encounter css loading timing issues, that is most likely due to use of deviceTheme in a deps list. Please see http://livedocs.dojotoolkit.org/dojox/mobile/deviceTheme

Changed 7 years ago by ykami

Attachment: ToolBarButton-fix.patch added

comment:17 Changed 7 years ago by ykami

BTW, I didn't mean to make defaultColor and selColor private. Actually they are used in test_Heading.html. I removed the private tags from their comments in the patch.

comment:18 Changed 7 years ago by cjolif

billreed63, can you give a try to ykami patch?

comment:19 Changed 7 years ago by cjolif

Status: newopen

comment:20 in reply to:  18 Changed 7 years ago by Bill Reed

Replying to cjolif:

billreed63, can you give a try to ykami patch?

The patch works!! Thanks..

comment:21 Changed 7 years ago by cjolif

Milestone: tbd1.8
Priority: undecidedhigh

comment:22 Changed 7 years ago by cjolif

Resolution: fixed
Status: openclosed

In [29312]:

fixes #15675. Thanks ykami.

comment:23 Changed 7 years ago by cjolif

Resolution: fixed
Status: closedreopened

The fix introduces two regressions on "compat" browser (IE and Firefox). So I'm re-opening this.

Changed 7 years ago by cjolif

Attachment: 15675-compatFixes.patch added

Patch for the issue from Kamiyama, Christophe and Adrian (IBM, CCLA)

comment:24 Changed 7 years ago by ykami

cjolif, I thought about those issues, and unfortunately now I think the patch (ToolBarButton-fix.patch) was not a very good idea. It introduces not only the compat problems you mentioned but also an iPad theme problem. We will need a special hard-code to avoid that. I also noticed it could cause a problem for dynamic theme switch. Perhaps something more... Can we revert the patch and reconsider it more carefully after 1.8.0?

comment:25 Changed 7 years ago by cjolif

Resolution: fixed
Status: reopenedclosed

In [29430]:

fixes #15675, fixes Maqetta issue without creating regressions this time (Thanks Kamiyama, Adrian & Damien). refs #14598, #15400. !strict.

Note: See TracTickets for help on using tickets.