+++ This bug was initially created as a clone of Bug #915074 +++ While working on bug 915074 I noticed that we have yet another important issue to fix regarding the call waiting UI for CDMA: according to the spec (attachment 803538 [details]) the "add call" button should be hidden but it's visible in the current implementation. I'm nom'ing for koi+ because this is an important change as that button does not work in CDMA mode and because the fix is real easy: I've got a patch almost ready for this and it's three lines of CSS and four of JS code so pretty low-risk.

This small patch adds functionality to hide the 'add call' button when in CDMA mode and also cleans up the CDMA CSS selectors to better match the file's style.

This screenshot shows the effect of this patch, note that the call toolbar now has three buttons instead of four as per spec (attachment 810350 [details]).

Comment on attachment 817941 [details] [diff] [review] [PATCH] Remove the 'add call' button when in CDMA call waiting mode Review of attachment 817941 [details] [diff] [review]: ----------------------------------------------------------------- It's also probably worth rebasing it on top of your landing spree from yesterday :) ::: apps/communications/dialer/js/call_screen.js @@ +58,5 @@ > }, > > set cdmaCallWaiting(enabled) { > + this.calls.classList.toggle('switch', enabled); > + this.callToolbar.classList.toggle('no-add-call', enabled); looks like you're missing a test :) ::: apps/communications/dialer/style/oncall.css @@ +214,4 @@ > border-right: 0; > } > > + #co-advanced.no-add-call span.grid-cell:nth-child(3) { wouldn't mind adding a id on this in the dom do make this more solid. @@ +378,5 @@ > display: none; > } > > + #calls.switch > section .switch-calls { > + display: block; background: #01c5ed; missing \n ?

(In reply to Etienne Segonzac (:etienne) from comment #3) > It's also probably worth rebasing it on top of your landing spree from > yesterday :) Right, this should apply cleanly :-) > ::: apps/communications/dialer/js/call_screen.js > @@ +58,5 @@ > > }, > > > > set cdmaCallWaiting(enabled) { > > + this.calls.classList.toggle('switch', enabled); > > + this.callToolbar.classList.toggle('no-add-call', enabled); > > looks like you're missing a test :) Actually I broke it, silly me :-( I've fixed the test and I've expanded it to include the second class. > ::: apps/communications/dialer/style/oncall.css > @@ +214,4 @@ > > border-right: 0; > > } > > > > + #co-advanced.no-add-call span.grid-cell:nth-child(3) { > > wouldn't mind adding a id on this in the dom do make this more solid. I've labeled the appropriate container with a new id so the CSS should be more solid and less obscure too. > @@ +378,5 @@ > > display: none; > > } > > > > + #calls.switch > section .switch-calls { > > + display: block; background: #01c5ed; > > missing \n ? Yup, thanks for the review!

Pointer to Github pull-request

I added a very small change to the unit tests description to reflect the changes in this patch.

triage: koi+ for a broken feature

Comment on attachment 818569 [details] [diff] [review] [PATCH v3] Remove the 'add call' button when in CDMA call waiting mode Review of attachment 818569 [details] [diff] [review]: ----------------------------------------------------------------- All good! Thanks!

Uplifted patch for v1.2, it did not require any modifications so carry over the r+ flag.

Pushed both patches: master e6b176ab1101577f80604aca0c3b3c3b58f74b03 v1.2 38e93cec6db6628f327568a88dc8ad7c719fca71

Verified on Wasabi Gaia: 00d5964eabf95a6a8a632420dfa36fc76dcbc9b7 Gecko: b42688ba6de51d7fbf5e9ec8fcc0d27e31b4134d BuildID 20131022031555 Version 26.0a2