-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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(android): Fixed bug where setting SearchBar "hintText" after window open crashes as of 7.0.0 #10848
fix(android): Fixed bug where setting SearchBar "hintText" after window open crashes as of 7.0.0 #10848
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -347,7 +347,10 @@ public void propertyChanged(String key, Object oldValue, Object newValue, KrollP | |
} else if (key.equals(TiC.PROPERTY_COLOR)) { | ||
tv.setTextColor(TiConvert.toColor((String) newValue)); | ||
} else if (key.equals(TiC.PROPERTY_HINT_TEXT)) { | ||
int type = proxy.getProperties().getInt(TiC.PROPERTY_HINT_TYPE); | ||
int type = UIModule.HINT_TYPE_STATIC; | ||
if (proxy.getProperties() != null) { | ||
type = TiConvert.toInt(proxy.getProperties().get(TiC.PROPERTY_HINT_TYPE), type); | ||
} | ||
setHintText(type, TiConvert.toString(newValue)); | ||
} else if (key.equals(TiC.PROPERTY_HINT_TEXT_COLOR)) { | ||
tv.setHintTextColor(TiConvert.toColor((String) newValue)); | ||
|
@@ -921,7 +924,11 @@ public void setAttributedStringHint(AttributedStringProxy attrString) | |
{ | ||
Spannable spannableText = AttributedStringProxy.toSpannable(attrString, TiApplication.getAppCurrentActivity()); | ||
if (spannableText != null) { | ||
int type = getProxy().getProperties().getInt(TiC.PROPERTY_HINT_TYPE); | ||
int type = UIModule.HINT_TYPE_STATIC; | ||
KrollProxy proxy = getProxy(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. another potential place we could return a null object variant? i.e. if a TiUIView's proxy is null, return a special NullProxy instance that has empty properties? |
||
if ((proxy != null) && (proxy.getProperties() != null)) { | ||
type = TiConvert.toInt(proxy.getProperties().get(TiC.PROPERTY_HINT_TYPE), type); | ||
} | ||
setHintText(type, spannableText); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
/* | ||
* Appcelerator Titanium Mobile | ||
* Copyright (c) 2011-Present by Appcelerator, Inc. All Rights Reserved. | ||
* Licensed under the terms of the Apache Public License | ||
* Please see the LICENSE included with this distribution for details. | ||
*/ | ||
/* eslint-env mocha */ | ||
/* global Ti */ | ||
/* eslint no-unused-expressions: "off" */ | ||
'use strict'; | ||
var should = require('./utilities/assertions'); | ||
|
||
describe('Titanium.UI.SearchBar', function () { | ||
var win; | ||
|
||
afterEach(function () { | ||
if (win) { | ||
win.close(); | ||
} | ||
win = null; | ||
}); | ||
|
||
it('Change hintText dynamically', function (finish) { | ||
this.timeout(5000); | ||
|
||
const OLD_HINT_TEXT = 'Old Hint Text'; | ||
const NEW_HINT_TEXT = 'New Hint Text'; | ||
const searchBar = Ti.UI.createSearchBar({ | ||
hintText: OLD_HINT_TEXT, | ||
}); | ||
should(searchBar.hintText).eql(OLD_HINT_TEXT); | ||
win = Ti.UI.createWindow(); | ||
win.add(searchBar); | ||
win.addEventListener('open', function () { | ||
try { | ||
should(searchBar.hintText).eql(OLD_HINT_TEXT); | ||
searchBar.hintText = NEW_HINT_TEXT; | ||
should(searchBar.hintText).eql(NEW_HINT_TEXT); | ||
} catch (err) { | ||
finish(err); | ||
return; | ||
} | ||
setTimeout(function () { | ||
try { | ||
should(searchBar.hintText).eql(NEW_HINT_TEXT); | ||
finish(); | ||
} catch (err) { | ||
finish(err); | ||
} | ||
}, 100); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't need to add any specific delay in milliseconds here, I wouldn't think. You could put 1 and it should basically allow the UI thread to dispatch before we fire. (also note that technically the delay arg is optional, and under the hood it enforces a minimum delay anyways - so maybe just omit the delay) |
||
}); | ||
win.open(); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assign to a temp var to avoid grabbing it via getter twice?
Also, we should consider using a null object pattern here to avoid the null check altogether. If a KrollProxy has a null
properties
field, can't we return an empty KrollDict and get the same behavior typically? (i.e. returns null for everything, returns default values if passed in)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to change this to use
proxy.getProperty(TiC.PROPERTY_HINT_TYPE)
instead which will returnnull
if the properties dictionary isnull
, but I can see this PR is already merged. That said, I think the current code is fine anyways.