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

feat: add "isTrusted" property to Slider "change" event #11717

Merged
merged 8 commits into from Jul 1, 2020

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented May 20, 2020

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

Summary:
Added a "isTrusted" boolean property to Ti.UI.Slider object's "change" event. Property will be true if change came from the end-user. Will be false if change was made programmatically. This is a standard JavaScript event property available to web developers. (In the future, we should apply this property to all events.)
https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted

Test:

  1. Build and run the below for Android.
  2. Drag the slider with your finger.
  3. In the log, verify change event's isTrusted property is set to true.
  4. Tap on the "Update Slider" button.
  5. In the log, verify change event's isTrusted property is set to false.
var window = Ti.UI.createWindow();
var slider = Ti.UI.createSlider({
	min: 0.0,
	max: 1.0,
	value: 0.25,
});
slider.addEventListener("change", function(e) {
	Ti.API.info("@@@ slider value changed: " + e.value + ", isTrusted: " + e.isTrusted);
});
window.add(slider);
var button = Ti.UI.createButton({
	title: "Update Slider Position",
	bottom: "20dp",
});
button.addEventListener("click", function() {
	slider.value = 0.5;
});
window.add(button);
window.open();

@m1ga m1ga changed the title feat(android): expose fromUser in Slider feat(android): expose fromUser in Slider onChange May 20, 2020
@build build added this to the 9.1.0 milestone May 20, 2020
@build build requested a review from a team May 20, 2020 11:18
@build
Copy link
Contributor

build commented May 20, 2020

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

Commit fae39e6a5cf54d88213ad7c3f0b9cf5c02358f65 has a message "change to isTrusted, docu" giving 2 errors:

  • subject may not be empty
  • type may not be empty
Messages
📖

💾 Here's the generated SDK zipfile.

📖 🎉 Another contribution from our awesome community member, m1ga! Thanks again for helping us make Titanium SDK better. 👍
📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

📖 ❌ 2 tests have failed There are 2 tests failing and 698 skipped out of 7347 total tests.

Tests:

ClassnameNameTimeError
android.emulator.Titanium.UI.TextArea.focused (9)2.297
Error: timeout of 2000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53723)
android.emulator.Titanium.UI.TextField.focused (9)2.328
Error: timeout of 2000ms exceeded
at Titanium.<anonymous> (/ti-mocha.js:6535:53723)

Generated by 🚫 dangerJS against cd85cfd

@build build requested a review from a team June 23, 2020 16:28
@build build added the docs label Jun 23, 2020
@jquick-axway
Copy link
Contributor

@m1ga, would you mind if I add changes to this PR?
I'd like to make the same change to iOS and add a unit test.

@m1ga
Copy link
Contributor Author

m1ga commented Jun 23, 2020

@jquick-axway no, go ahead!

@jquick-axway jquick-axway changed the title feat(android): expose fromUser in Slider onChange feat: add "isTrusted" property to Slider "change" event Jun 24, 2020
@jquick-axway jquick-axway requested review from janvennemann and removed request for a team June 24, 2020 00:12
@jquick-axway
Copy link
Contributor

PR updated:

  • Added property to iOS for parity.
  • Added unit test.
  • Updated API docs.

@janvennemann, would you mind reviewing the iOS side? Do you have any objections to this new event property?

Copy link
Contributor

@janvennemann janvennemann left a comment

Choose a reason for hiding this comment

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

@jquick-axway iOS change looks good. I'm fine with introducing this now since we already fire the event on programmatic changes. Eventually we should stop firing the change event after programmatic changes though, but as already discussed we can deal with that in a separate ticket for an upcoming major release.

Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed Tested using the test case and instructions mentioned above in the description.

Test Environment

MacOS Big Sur: 11.0 Beta
Xcode: 12.0 Beta 
Java Version: 1.8.0_242
Android NDK: 21.3.6528147
Node.js: 12.18.1
""NPM":"5.0.0","CLI":"8.0.0""
iphone 8 Sim (14.0 Beta)
API29 Pixel XL emulator

@sgtcoolguy sgtcoolguy merged commit 8e96445 into tidev:master Jul 1, 2020
@ssaddique
Copy link
Contributor

Fix verified on build 9.1.0.v20200804082025.

Test environment

OS Ver:         10.15.3
Xcode Ver:      Xcode 11.6
Appc NPM:       5.0.0
Appc CLI:       8.0.0
Node Ver:       10.17.0
Java Ver:       1.8.0_162
Devices:        iPhone 6 (12.1.4), Pixel 3a (11.0)

I must note that when clicking the 'Update' button, the value is logged twice:

[INFO] @@@ slider value changed: 0.5, isTrusted: false
[INFO] @@@ slider value changed: 0.5, isTrusted: false

@m1ga m1ga deleted the sliderEvent branch August 7, 2020 07:53
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

7 participants