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

fix(iOS): Ti.UI.convertUnits does not consult ti.ui.defaultunit property #10462

Closed
wants to merge 3 commits into from

Conversation

sgtcoolguy
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-26113

Description:
When attempting to convert units, the Ti.UI.convertUnits method should assume the units specified in the ti.ui.defaultunit Ti.App.Property value when the first value passed in does not explicitly state the input units.

This PR fixes this on iOS. Under the hood is didn't consult it at all and instead assumed 'dpi'. I also made that method allow for number values to be passed in for the first argument (and not just strings).

Note that the first commit is simply to fix the formatting on the files to match our standards, you should look at the second commit for review purposes.

Test case:
In your tiapp.xml file, change the ti.ui.defaultunit value to 'px' or 'in' for the tests below. Our automated suite uses whatever unit is filled into the trap.xml by default, which is 'dp' - so I can't easily write an automated test for this fix (as 'dp' and 'dpi' are just alternate ways of specifying the same unit type, and so our default apps fill in the same unit type for default as the iOS was hard-coded to assume).

const logicalDensityFactor = Ti.Platform.displayCaps.logicalDensityFactor;
const dpi = Ti.Platform.displayCaps.dpi;
// When ti.ui.defaultunit is px, test value as Number and String
Ti.API.info('should be roughly 2.54: ' + Ti.UI.convertUnits(dpi + '', Ti.UI.UNIT_CM));
Ti.API.info('should be roughly 2.54: ' + Ti.UI.convertUnits(dpi, Ti.UI.UNIT_CM));
Ti.API.info('should be roughly 1: ' + Ti.UI.convertUnits(logicalDensityFactor + '', Ti.UI.UNIT_DIP));
Ti.API.info('should be roughly 1: ' + Ti.UI.convertUnits(logicalDensityFactor, Ti.UI.UNIT_DIP));
Ti.API.info('should be exactly 1: ' + Ti.UI.convertUnits(dpi + '', Ti.UI.UNIT_IN));
Ti.API.info('should be exactly 1: ' + Ti.UI.convertUnits(dpi, Ti.UI.UNIT_IN));
Ti.API.info('should be roughly 25.4: ' + Ti.UI.convertUnits(dpi + '', Ti.UI.UNIT_MM));
Ti.API.info('should be roughly 25.4: ' + Ti.UI.convertUnits(dpi, Ti.UI.UNIT_MM));

// When ti.ui.defaultunit is in, test value as Number and String
Ti.API.info('should be roughly 2.54: ' + Ti.UI.convertUnits('1', Ti.UI.UNIT_CM));
Ti.API.info('should be roughly 2.54: ' + Ti.UI.convertUnits(1, Ti.UI.UNIT_CM));
Ti.API.info('should be roughly ' + (dpi / logicalDensityFactor) + ': ' + Ti.UI.convertUnits('1', Ti.UI.UNIT_DIP));
Ti.API.info('should be roughly ' + (dpi / logicalDensityFactor) + ': ' + Ti.UI.convertUnits(1, Ti.UI.UNIT_DIP));
Ti.API.info('should be roughly ' + dpi + ': ' + Ti.UI.convertUnits('1', Ti.UI.UNIT_PX));
Ti.API.info('should be roughly ' + dpi + ': ' + Ti.UI.convertUnits(1, Ti.UI.UNIT_PX));

@build
Copy link
Contributor

build commented Nov 13, 2018

Fails
🚫

Tests have failed, see below for more information.

Messages
📖

👍 Hey!, You deleted more code than you added. That's awesome!

📖

💾 Here's the generated SDK zipfile.

Tests:

Classname Name Time Error
android.Titanium.UI.TextField focus-blur-bubbles 5.873 timeout of 5000ms exceeded

Generated by 🚫 dangerJS

Copy link
Contributor

@vijaysingh-axway vijaysingh-axway left a comment

Choose a reason for hiding this comment

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

CR passed.

sgtcoolguy added a commit to sgtcoolguy/titanium_mobile that referenced this pull request Nov 14, 2018
…ons when input units aren't specified.

feat(ios): Support passing in numbers to Ti.UI.convertUnits()

Fixes TIMOB-26113 (tidev#10462)
@sgtcoolguy sgtcoolguy closed this Nov 14, 2018
@sgtcoolguy sgtcoolguy deleted the TIMOB-26113 branch November 14, 2018 15:35
@sgtcoolguy
Copy link
Contributor Author

Merged manually

vijaysingh-axway pushed a commit to vijaysingh-axway/titanium_mobile that referenced this pull request Nov 22, 2018
…ons when input units aren't specified.

feat(ios): Support passing in numbers to Ti.UI.convertUnits()

Fixes TIMOB-26113 (tidev#10462)
build pushed a commit to hansemannn/titanium_mobile that referenced this pull request Dec 3, 2018
…ons when input units aren't specified.

feat(ios): Support passing in numbers to Ti.UI.convertUnits()

Fixes TIMOB-26113 (tidev#10462)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants