Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timob 7799 Removed Flexbox #1885

Merged
merged 19 commits into from Apr 13, 2012
Merged

Timob 7799 Removed Flexbox #1885

merged 19 commits into from Apr 13, 2012

Conversation

nebrius
Copy link
Contributor

@nebrius nebrius commented Mar 29, 2012

No description provided.

width = lang.val(this.width,this._defaultWidth),
height = lang.val(this.height,this._defaultHeight);
(isDef(this.width) || !widthOverride) && (isWidthSize = (width === UI.INHERIT ? this._getInheritedWidth() : width) === UI.SIZE);
(isDef(this.width) || !heightOverride) && (isHeightSize = (width === UI.INHERIT ? this._getInheritedWidth() : width) === UI.SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an awful lot of widths on this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on the bug. It slipped my testing somehow.

return value;
},
value: UI.TEXT_VERTICAL_ALIGNMENT_CENTER
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing comma!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@cb1kenobi
Copy link
Contributor

You missed a textContainerDiv in Label in _setTextShadow() that is breaking drop shadows.

@cb1kenobi
Copy link
Contributor

I created a ticket TIMOB-8447 to remove all returns from all property post functions. Could you remove the returns from all post functions from the files touched in this PR? I know there's some in Label.

@nebrius
Copy link
Contributor Author

nebrius commented Mar 30, 2012

Done.

var text = self._textContainerDomNode.innerHTML;
return {
width: self._measureText(text, textContainerDomNode, width).width,
height: self._measureText(text, textContainerDomNode, width).height
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be "height", not "width".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it's actually correct. Width (inside the parenthesis) is used to specify a maximum width of the label, i.e. it's used when you want to measure the height of wrapped text.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, cool.

@cb1kenobi
Copy link
Contributor

There are still a couple of blockers preventing this PR from being merged. The following are generally issues that weren't issues prior to this PR.

  1. Ti.UI.Button text alignment and size is not correct in Firefox. In certain cases, text is not vertically or horizontally aligned properly. The size of the back button in the nav group is not correct either. Seems to only affect Firefox.

  2. Text and the on/off indicator in a Ti.UI.Switch is not correct in Firefox. It cuts off nearly all of the indicator. Seems to only affect Firefox.

  3. Labels with left aligned text does not left align. Suspect same issue with right aligned text.

  4. Icons on tabs do not display on an iphone until you rotate to force a layout recalculation. Works fine in desktop browsers and only appears to happen on device. Not sure about Android devices.

@cb1kenobi
Copy link
Contributor

Code reviewed and tested. Request accepted.

cb1kenobi added a commit that referenced this pull request Apr 13, 2012
@cb1kenobi cb1kenobi merged commit ff133a3 into tidev:master Apr 13, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants