- 
                Notifications
    You must be signed in to change notification settings 
- Fork 21
fix(playground): set auto run on clear #669
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
Conversation
| @OnkarRuikar Thank you for taking the time to work on these changes. For all the issues that you're addressing in this PR, and that don't have an issue yet, can you please create one? I think they should first go through triage, before we can accept fixes. That said, if you could extract the fixes for #663 into a separate PR, that would be great to land that fix earlier. | 
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.
Skimmed over the changes, and left some comments here and there.
        
          
                components/playground/element.js
              
                Outdated
          
        
      | this._reportToastHide(); | ||
| controller.clear(); | ||
| this._autoRun = true; | ||
| controller.runOnChange = true; | 
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.
This should probably happen as part of controller.clear().
| controller.runOnChange = true; | 
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.
done
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.
done
| 
 Created separate issues: 
 
 Let's fix #663 here itself and rest of the stuff in separate PRs after triages there. | 
8ce0d70    to
    83de4d7      
    Compare
  
    
Issues
The PR addresses the following issues in the playground:
2. After clearing, the report message(Seeing something inappropriate?) toast doesn't go away.3. The report message doesn't go away after closing the report dialog.4. There is no close/dismiss button on the report toast/banner.Solution
_autoRunandrunOnChangevariables.- For the second issue, use${this._gistId && this._showReportToastcondition and hide the toast message on clear button click.- For the third and fourth issues, define a new toast element named MDNToast to show dismissable messages. Toasts are lightweight, dismissible notifications.Related issues and pull requests
Fix #663