Closed
Bug 992193
Opened 10 years ago
Closed 10 years ago
[Dialer] USSD notify/request specific UI management
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(tracking-b2g:backlog, b2g-v2.0 fixed)
Tracking | Status | |
---|---|---|
b2g-v2.0 | --- | fixed |
People
(Reporter: noemi, Assigned: fcampo)
References
Details
(Whiteboard: [priority])
Attachments
(3 files)
Currently both types of USSD codes are handled very similar in terms of UI, so no matter if the network is waiting for a response (USSD request, please see "Current_USSD-request_UI.png) or not (USSD notify, please see "Current_USSD-notify_UI.png") we always show "Send" button in the header, in the first case "Send" button will be activated when typing something in the input field in the second case "Send" button will be always shown as deactivated. The proposal would be to remove "Send" button for USSD notify case, so we will apply the most appropriated UI for each case. ni to UX to confirm the right path to be followed here.
Reporter | ||
Comment 1•10 years ago
|
||
Reporter | ||
Updated•10 years ago
|
Flags: needinfo?(cawang)
Reporter | ||
Comment 2•10 years ago
|
||
Adding to backlog to be properly prioritized. Thanks!
blocking-b2g: --- → backlog
Comment 3•10 years ago
|
||
Hi Noemi, Yes, I've faced this case before and felt quite confused. I agree with your proposal here. The send button in the case of USSD notify should be removed. Thanks!
Flags: needinfo?(cawang)
Updated•10 years ago
|
Whiteboard: [priority]
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → fernando.campo
Assignee | ||
Comment 4•10 years ago
|
||
Attachment #8422450 -
Flags: review?(etienne)
Comment 5•10 years ago
|
||
Comment on attachment 8422450 [details] [review] Link to PR - https://github.com/mozilla-b2g/gaia/pull/19247 Hey, I know we don't have a mmi_ui_test.js yet, but I think we should be able to cover this change with unit test pretty cheaply. And since we have some changes/refactorings coming to this code, we definitely should! :)
Attachment #8422450 -
Flags: review?(etienne)
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #5) > Comment on attachment 8422450 [details] [review] > Link to PR - https://github.com/mozilla-b2g/gaia/pull/19247 > > Hey, I know we don't have a mmi_ui_test.js yet, but I think we should be > able to cover this change with unit test pretty cheaply. > > And since we have some changes/refactorings coming to this code, we > definitely should! :) Thing is, mmi_test.js is using MockMmiUI (heavily dependance, actually), which doesn't let me test the calls to the real ui. So we have some options here: - By creating a mmi_ui_tests.js I could test that the calls to the functions trigger the proper result, if that is enough at the moment. - Other option (ideal) would be to use mmi_tests.js to test both functionality and UI changes, but that would take me some time, to stop using the mock and rely on the real_ui. Having in mind that this is a small fix, I'd go for the first option, create a quick mmi_ui_test file and test that a mock_message received triggers the proper UI changes. WDYT Etienne?
Flags: needinfo?(etienne)
Comment 7•10 years ago
|
||
(In reply to Fernando Campo (:fcampo) from comment #6) > (In reply to Etienne Segonzac (:etienne) from comment #5) > > Comment on attachment 8422450 [details] [review] > > Link to PR - https://github.com/mozilla-b2g/gaia/pull/19247 > > > > Hey, I know we don't have a mmi_ui_test.js yet, but I think we should be > > able to cover this change with unit test pretty cheaply. > > > > And since we have some changes/refactorings coming to this code, we > > definitely should! :) > Thing is, mmi_test.js is using MockMmiUI (heavily dependance, actually), > which doesn't let me test the calls to the real ui. So we have some options > here: > - By creating a mmi_ui_tests.js I could test that the calls to the functions > trigger the proper result, if that is enough at the moment. > - Other option (ideal) would be to use mmi_tests.js to test both > functionality and UI changes, but that would take me some time, to stop > using the mock and rely on the real_ui. > > Having in mind that this is a small fix, I'd go for the first option, create > a quick mmi_ui_test file and test that a mock_message received triggers the > proper UI changes. WDYT Etienne? Yeah the first option is exactly what I had in mind, we agree :)
Flags: needinfo?(etienne)
Assignee | ||
Comment 8•10 years ago
|
||
Having some problems with the test (postMessage and mocha). After discussing on IRC with Julien, probably the best option is to upgrade mocha (bug 874510) instead of making workarounds in here.
Depends on: 874510
Comment 9•10 years ago
|
||
Did you try doing a |new CustomEvent('message')|?
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Etienne Segonzac (:etienne) from comment #9) > Did you try doing a |new CustomEvent('message')|? Yeah, tried customEvent, messageEvent after Julien's advice. Also run the tests inside a eventListener (Rik's tip). Every time, same thing, as the tested action happens after a 'message' event received, and Mocha uses postMessage for the ticks, we receive a lot more events than we should, and test never goes complete :(
Comment 11•10 years ago
|
||
(In reply to Fernando Campo (:fcampo) from comment #10) > (In reply to Etienne Segonzac (:etienne) from comment #9) > > Did you try doing a |new CustomEvent('message')|? > Yeah, tried customEvent, messageEvent after Julien's advice. > Also run the tests inside a eventListener (Rik's tip). > Every time, same thing, as the tested action happens after a 'message' event > received, and Mocha uses postMessage for the ticks, we receive a lot more > events than we should, and test never goes complete :( :/ New suggestion (from irc) > and if you don't init the MmiUI and call handleEvent({message}) from the test ? :)
Comment 12•10 years ago
|
||
This works: test('Response is hidden', function(done) { window.addEventListener('message', function(evt) { // Bug xyz - Mocha uses message event so we're filtering them out if (evt.data.type != 'mmi-success') { return; } assert.isTrue(MmiUI.hideResponseForm.called); assert.isTrue(MmiUI.sendNode.classList.contains('hide')); assert.isFalse(MmiUI.mmiScreen.classList.contains('responseForm')); done(); }); });
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8422450 [details] [review] Link to PR - https://github.com/mozilla-b2g/gaia/pull/19247 Rik comments added, tests working (with comments inside, as we can't test everything yet). Hopefully enough for landing while we wait for bug 874510
Attachment #8422450 -
Flags: review?(etienne)
Comment 14•10 years ago
|
||
Comment on attachment 8422450 [details] [review] Link to PR - https://github.com/mozilla-b2g/gaia/pull/19247 kudos!
Attachment #8422450 -
Flags: review?(etienne) → review+
Assignee | ||
Comment 15•10 years ago
|
||
merged on master - 1fe6e6f9518ae3cf57f374048955d209e9b797da Thanks all for the help with the tests!
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
status-b2g-v2.0:
--- → fixed
Target Milestone: --- → 2.0 S2 (23may)
Comment 16•10 years ago
|
||
Tested and working. Hamachi 2.0 Gecko-1a316f5 Gaia-deed49c - "Send" button not appear in ussd-notify case. - "Send" button appear disabled in ussd-menú case and any option isn't selected. - "Send" button appear enabled in ussd-menú case and any option is selected.
Status: RESOLVED → VERIFIED
Updated•9 years ago
|
blocking-b2g: backlog → ---
tracking-b2g:
--- → backlog
You need to log in
before you can comment on or make changes to this bug.
Description
•