1175441 - 'Let`s talk about' context tickbox default state

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect)

Description

[Bogdan Maris, Desktop QA]

Affected builds:
- Latest Nightly 41.0a1
- Latest Aurora 40.0a2

Affected OS`s:
- Windows 7 64-bit
- Mac OS X 10.9.5
- Ubuntu 14.04 32-bit

STR:
1. Start Firefox
2. Visit a webpage (eg. https://www.mozilla.org/en-US/firefox/hello/)
3. Click hello button
4. Check 'Let`s talk about'
5. Start conversation
6. Close conversation
7. Visit another webpage (eg. https://www.wikipedia.org/)
8. Click hello button

Expected results: 'Let`s talk about' tickbox is not ticked.

Actual results: Tickbox is ticked by default in that session. After restarting firefox the tickbox is not ticked by default.

Notes:
- Not sure about the intended behavior here, requesting needinfo from sevaan on this.

Comment 1

[Sevaan Franks [:sevaan]]

Bogdan's expectations are correct. The box should be unchecked when you go to a new page.

Comment 2

[Bogdan Maris, Desktop QA]

(In reply to Sevaan Franks [:sevaan] from comment #1)
> Bogdan's expectations are correct. The box should be unchecked when you go
> to a new page.

Thanks for your input on this! cc-ing Standard, Mike and shell.

Comment 3

[Mark Banner (:standard8)]

Attachment: Clear the 'Let's talk about' tickbox in the Loop panel when the page selection changes.

This makes us clear the checkbox whenever we get a new page returned.

Comment 4

[Mike de Boer [:mikedeboer]]

Comment on attachment 8624139 [details] [diff] [review]
Clear the 'Let's talk about' tickbox in the Loop panel when the page selection changes.

Review of attachment 8624139 [details] [diff] [review]:
-----------------------------------------------------------------

Good stuff, I like it! :)

::: browser/components/loop/content/shared/js/views.jsx
@@ +636,5 @@
> +    componentWillReceiveProps: function(nextProps) {
> +      // Only change the state if the prop has changed, and if it is also
> +      // different from the state.
> +      if (this.props.checked != nextProps.checked &&
> +          this.state.checked != nextProps.checked) {

I think you want strict equality checking here.

@@ +637,5 @@
> +      // Only change the state if the prop has changed, and if it is also
> +      // different from the state.
> +      if (this.props.checked != nextProps.checked &&
> +          this.state.checked != nextProps.checked) {
> +        this.setState({checked: nextProps.checked});

nit: object literal formatting.

::: browser/components/loop/test/desktop-local/panel_test.js
@@ +860,5 @@
> +      };
> +
> +      var view = createTestComponent();
> +
> +      view.setState({checked: true});

nit: object literal formatting.

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/c150d89825a6

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c150d89825a6

Comment 7

[Bogdan Maris, Desktop QA]

Verified that the issue is fixed using Latest Aurora 41.0a2 across platforms (Windows 7 64-bit, Mac OS X 10.10 and Ubuntu 14.04 32-bit).