Opened 9 years ago
Closed 8 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)
Change History (30)
comment:1 Changed 9 years ago by
Cc: | Adam Peller added |
---|
comment:2 Changed 9 years ago by
Cc: | cjolif added |
---|
comment:3 Changed 9 years ago by
Cc: | Damien Mandrioli added |
---|
comment:5 Changed 9 years ago by
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.
comment:6 Changed 9 years ago by
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 9 years ago by
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 9 years ago by
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.
comment:9 Changed 9 years ago by
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.
comment:10 Changed 9 years ago by
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.
- call _updateArrowColor() after applying a custom color to the button body.
- prepare a calculated style for arrow, and apply it to the arrow when you change the body color.
comment:11 Changed 9 years ago by
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 9 years ago by
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....
comment:13 follow-up: 14 Changed 9 years ago by
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.
comment:14 Changed 9 years ago by
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 9 years ago by
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 9 years ago by
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 9 years ago by
Attachment: | ToolBarButton-fix.patch added |
---|
comment:17 Changed 9 years ago by
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:19 Changed 9 years ago by
Status: | new → open |
---|
comment:20 Changed 9 years ago by
comment:21 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|---|
Priority: | undecided → high |
comment:23 Changed 8 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
The fix introduces two regressions on "compat" browser (IE and Firefox). So I'm re-opening this.
Changed 8 years ago by
Attachment: | 15675-compatFixes.patch added |
---|
Patch for the issue from Kamiyama, Christophe and Adrian (IBM, CCLA)
comment:24 Changed 8 years ago by
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?
Eric, sorry for the late notice, but if we could have this addressed in time for 1.8, that would be helpful