Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2568283003:
bluetooth: Add notification system to internals page. (Closed)
Descriptionbluetooth: Add notification system to internals page.
Adds notification system to internals page using a snackbar UI element.
The snackbar system can be invoked from any page using Snackbar.show and
Snackbar.dismiss and come in a variety of styles (info, success, warning, and
danger) depending on the need of the caller. Multiple snackbar requests can be
queued and are shown one at a time.
Removes connection error column from DeviceTable.
Error and success messages are now shown in a snackbar displayed from
bluetooth_internals.js.
Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47
GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5
BUG=663830
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/22fa9c78d47051c4da59d68e38b2e6bd9b76f880
Cr-Commit-Position: refs/heads/master@{#439963}
Patch Set 1 #Patch Set 2 : Fix up snackbar, add tests #Patch Set 3 : Add basic test #Patch Set 4 : Update comments #Patch Set 5 : Fix snackbar bug with contentfocus/contentblur #
Total comments: 6
Patch Set 6 : Change handleInspect, danger->error, fix text color #
Total comments: 22
Patch Set 7 : CSS/Snackbar/test changes #
Total comments: 17
Patch Set 8 : Fix JSDoc, remove window from tests, change snackbar margin #Patch Set 9 : Remove margin, fix bug where snackbar dismisses if shown with no focus #Dependent Patchsets: Messages
Total messages: 42 (29 generated)
Description was changed from ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 BUG=663830 ========== to ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
The CQ bit was checked by mbrunson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mbrunson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mbrunson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
mbrunson@chromium.org changed reviewers: + scheib@chromium.org
LGTM, small notes below. Things I can't answer: - details of CSS in particular - if webui team supports building this component, if they had another component that would work that would take less code for this page. https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.js (right): https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/bluetooth_internals.js:126: devicesPage.pageDiv.addEventListener('inspectpressed', function(event) { why not just addEventListener('inspectpressed', handleInspect); https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/snackbar.js:30: DANGER: 'danger', danger seems strong. ;) 'error' perhaps? https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/snackbar.js:56: this.addEventListener('mouseleave', this.startTimeout_.bind(this)); these pairs seem reversed. Timer should run when user is 'paying attention', and so on 'mouseenter', right?
https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.js (right): https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/bluetooth_internals.js:126: devicesPage.pageDiv.addEventListener('inspectpressed', function(event) { On 2016/12/14 06:06:43, scheib wrote: > why not just > addEventListener('inspectpressed', handleInspect); Done. https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/snackbar.js:30: DANGER: 'danger', On 2016/12/14 06:06:43, scheib wrote: > danger seems strong. ;) 'error' perhaps? Done. https://codereview.chromium.org/2568283003/diff/80001/chrome/browser/resource... chrome/browser/resources/bluetooth_internals/snackbar.js:56: this.addEventListener('mouseleave', this.startTimeout_.bind(this)); On 2016/12/14 06:06:43, scheib wrote: > these pairs seem reversed. Timer should run when user is 'paying attention', and > so on 'mouseenter', right? The timer is for hiding the snackbar. The snackbar shouldn't disappear while someone is interacting with it, so the timer is stopped on mouseenter and restarted on mouseleave.
The CQ bit was checked by mbrunson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
mbrunson@chromium.org changed reviewers: + dbeam@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:210: /** Snackbar */ nit: /** is for jsdoc /* Snackbar */ is fine i suppose, but is this adding anything more than ... a class named .snackbar right below this comment? https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:217: font-size: 14pt; in theory font-size should be in scalable units, i.e. % or em https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/sidebar.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/sidebar.js:48: document.dispatchEvent(new CustomEvent('contentfocus')); why is this not named something like 'sidebar-close'? https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:13: * actionText: string|undefined, I think you need parens around, i.e. (string|undefined) https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:76: } no curlies, also: this.actionLink_.textContent = options.actionText || 'Dismiss'; is probably equivalent https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:84: this.actionLink_.addEventListener('click', this.dismiss.bind(this)); nit: this.actionLink_.addEventListener('click', function() { if (options.action) options.action(); this.dismiss(); }.bind(this)); https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:104: } nit: no curlies https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:116: this.timeoutFn_ = setTimeout(function() { timeoutFn_ -> startTimeoutInterval_? this type is actually a number, not a function, so naming it Fn_ is a little weird to me https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:134: /** @private {Snackbar} */ nit: ?Snackbar https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:138: Snackbar.queue_ = []; do you actually need to queue these right now? where can (and does) this happen? https://codereview.chromium.org/2568283003/diff/100001/chrome/test/data/webui... File chrome/test/data/webui/bluetooth_internals_browsertest.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/test/data/webui... chrome/test/data/webui/bluetooth_internals_browsertest.js:267: window.snackbar.Snackbar.dismiss(true); why do you need the "window."?
https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:210: /** Snackbar */ On 2016/12/16 08:42:58, Dan Beam wrote: > nit: /** is for jsdoc > > /* Snackbar */ is fine i suppose, but is this adding anything more than ... a > class named .snackbar right below this comment? Since there are a lot of CSS rules in this file, this comment just serves as a marker for the start of the Snackbar CSS. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:217: font-size: 14pt; On 2016/12/16 08:42:58, Dan Beam wrote: > in theory font-size should be in scalable units, i.e. % or em Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/sidebar.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/sidebar.js:48: document.dispatchEvent(new CustomEvent('contentfocus')); On 2016/12/16 08:42:58, Dan Beam wrote: > why is this not named something like 'sidebar-close'? The main purpose of these 2 events (contentfocus/contentblur) is to alert listeners that some UI element is going to block the main content of the page. I wanted it to be named something generic in case other UI elements are added that also block the user from accessing the main content (alert overlays, dialog boxes, etc.). The main use of this is to prevent the snackbar from closing before the user has a chance to read it and respond. I can use specific event names if you think that sounds like a bad idea. But I would have to have the Snackbar know about and subscribe to each event separately which seems like a more complex way of doing this. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:13: * actionText: string|undefined, On 2016/12/16 08:42:58, Dan Beam wrote: > I think you need parens around, i.e. (string|undefined) Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:76: } On 2016/12/16 08:42:58, Dan Beam wrote: > no curlies, also: > > this.actionLink_.textContent = options.actionText || 'Dismiss'; > > is probably equivalent Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:84: this.actionLink_.addEventListener('click', this.dismiss.bind(this)); On 2016/12/16 08:42:58, Dan Beam wrote: > nit: > > this.actionLink_.addEventListener('click', function() { > if (options.action) options.action(); > this.dismiss(); > }.bind(this)); Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:104: } On 2016/12/16 08:42:58, Dan Beam wrote: > nit: no curlies Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:116: this.timeoutFn_ = setTimeout(function() { On 2016/12/16 08:42:58, Dan Beam wrote: > timeoutFn_ -> startTimeoutInterval_? this type is actually a number, not a > function, so naming it Fn_ is a little weird to me Ah yes. Since setTimeout returns an ID, I should call this timeoutId_. Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:134: /** @private {Snackbar} */ On 2016/12/16 08:42:58, Dan Beam wrote: > nit: ?Snackbar Done. https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:138: Snackbar.queue_ = []; On 2016/12/16 08:42:58, Dan Beam wrote: > do you actually need to queue these right now? where can (and does) this > happen? A user may attempt to inspect multiple devices at the same time. Since connection requests are processed asynchronously, multiple errors (or success messages) could occur (one for each device). In that case, notifications will have to be queued. handleInspect code: https://codereview.chromium.org/2568283003/diff/100001/chrome/browser/resourc... https://codereview.chromium.org/2568283003/diff/100001/chrome/test/data/webui... File chrome/test/data/webui/bluetooth_internals_browsertest.js (right): https://codereview.chromium.org/2568283003/diff/100001/chrome/test/data/webui... chrome/test/data/webui/bluetooth_internals_browsertest.js:267: window.snackbar.Snackbar.dismiss(true); On 2016/12/16 08:42:58, Dan Beam wrote: > why do you need the "window."? Removed. Done.
https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:215: color: #F1F1F1; nit: html/css style guide generally prefers lower-case hex colors i think (i.e. #f1f1f1) https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:220: margin: 0 auto; could we change this to margin: 12px; to position like paper-toast? https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:245: min-width: unset; rather than setting and unsetting, maybe just set in a @media query? https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:274: transform: translate3d(0, 0, 0); if 2 snackbars show at once, does the more recent one cover the other? https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:15: * }} arguable nit: /** * @typedef {{ * message: string, * type: string, * actionText: (string|undefined), * action: (function()|undefined) * }} */ also: are you compiling this code? https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:153: Snackbar.queue_.push(newSnackbar); ah, so these are queued until others are dismissed. ok https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:158: return newSnackbar; nit: if (Snackbar.current_) Snackbar.queue_.push(newSnackbar); else Snackbar.show_(newSnackbar); return newSnackbar; https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:167: $('snackbar-container').appendChild(newSnackbar); probably OK for now, but maybe pass $('snackbar-container') as the place to insert the new snackbar instead of hardcoding the ID here? maybe just a TODO if we want to make this code reusable in the future (we might!). https://codereview.chromium.org/2568283003/diff/120001/chrome/test/data/webui... File chrome/test/data/webui/bluetooth_internals_browsertest.js (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/test/data/webui... chrome/test/data/webui/bluetooth_internals_browsertest.js:475: expectFalse(!!window.snackbar.Snackbar.current_); nit: remove "window." everywhere
this is looking pretty sweet, btw :)
https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/bluetooth_internals.css (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:215: color: #F1F1F1; On 2016/12/16 21:54:40, Dan Beam wrote: > nit: html/css style guide generally prefers lower-case hex colors i think (i.e. > #f1f1f1) Hmm ok. I fixed them all here. Done. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:220: margin: 0 auto; On 2016/12/16 21:54:40, Dan Beam wrote: > could we change this to margin: 12px; to position like paper-toast? I was wondering why they have that margin. The material design spec for snackbars doesn't specify that. I guess there's a style difference between toasts and snackbars. I had to move some things around, but it's like paper-toast now. Done. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:245: min-width: unset; On 2016/12/16 21:54:40, Dan Beam wrote: > rather than setting and unsetting, maybe just set in a @media query? Done. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/bluetooth_internals.css:274: transform: translate3d(0, 0, 0); On 2016/12/16 21:54:40, Dan Beam wrote: > if 2 snackbars show at once, does the more recent one cover the other? The most recent one would be on top since they're appended to #snackbar-container and have the same z index. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... File chrome/browser/resources/bluetooth_internals/snackbar.js (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:15: * }} On 2016/12/16 21:54:40, Dan Beam wrote: > arguable nit: > > /** > * @typedef {{ > * message: string, > * type: string, > * actionText: (string|undefined), > * action: (function()|undefined) > * }} > */ > > also: are you compiling this code? No. I'm not compiling it. IIRC, WebUI pages that use Mojo had an issue with the closure compiler. But I need to look into that again to make sure. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:158: return newSnackbar; On 2016/12/16 21:54:40, Dan Beam wrote: > nit: > > if (Snackbar.current_) > Snackbar.queue_.push(newSnackbar); > else > Snackbar.show_(newSnackbar); > > return newSnackbar; Done. https://codereview.chromium.org/2568283003/diff/120001/chrome/browser/resourc... chrome/browser/resources/bluetooth_internals/snackbar.js:167: $('snackbar-container').appendChild(newSnackbar); On 2016/12/16 21:54:40, Dan Beam wrote: > probably OK for now, but maybe pass $('snackbar-container') as the place to > insert the new snackbar instead of hardcoding the ID here? maybe just a TODO if > we want to make this code reusable in the future (we might!). Added a TODO. Done. https://codereview.chromium.org/2568283003/diff/120001/chrome/test/data/webui... File chrome/test/data/webui/bluetooth_internals_browsertest.js (right): https://codereview.chromium.org/2568283003/diff/120001/chrome/test/data/webui... chrome/test/data/webui/bluetooth_internals_browsertest.js:475: expectFalse(!!window.snackbar.Snackbar.current_); On 2016/12/16 21:54:41, Dan Beam wrote: > nit: remove "window." everywhere Done.
fwiw: I'm not sure I understood the difference between tiara and snackbars. maybe this shouldn't have a margin? either way, lgtm
The CQ bit was checked by mbrunson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mbrunson@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org, dbeam@chromium.org Link to the patchset: https://codereview.chromium.org/2568283003/#ps160001 (title: "Remove margin, fix bug where snackbar dismisses if shown with no focus")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1482282957827610,
"parent_rev": "6f7c4e18f4b435eca8c31af0daf15f53162d85a5", "commit_rev":
"c50a48b74dea8e73fd7fc4e4e8b8f218cd626470"}
Message was sent while issue was closed.
Description was changed from ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2568283003 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2568283003 ========== to ========== bluetooth: Add notification system to internals page. Adds notification system to internals page using a snackbar UI element. The snackbar system can be invoked from any page using Snackbar.show and Snackbar.dismiss and come in a variety of styles (info, success, warning, and danger) depending on the need of the caller. Multiple snackbar requests can be queued and are shown one at a time. Removes connection error column from DeviceTable. Error and success messages are now shown in a snackbar displayed from bluetooth_internals.js. Screenshots: https://goo.gl/photos/p7fb2QH3e7UpcZA47 GIFs: https://goo.gl/photos/9PWexXFZ6prr4uVq5 BUG=663830 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/22fa9c78d47051c4da59d68e38b2e6bd9b76f880 Cr-Commit-Position: refs/heads/master@{#439963} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/22fa9c78d47051c4da59d68e38b2e6bd9b76f880 Cr-Commit-Position: refs/heads/master@{#439963} |
