Add a support status banner to the admin dashboard
Review Request #8064 — Created March 17, 2016 and submitted
The support banner shows the current support status, via
beanbaginc.com's API. While the data is being retrieved, a spinner is
being shown.
Tested the banner with:
- an active support contract;
- an inactive support contract; and
- no support contract.
Description | From | Last Updated |
---|---|---|
Style-wise, it'd be nice to actually have this appear like a draft banner. Something flush against the left, top, and … |
chipx86 | |
Undo this change. |
david | |
Add another blank line here. |
david | |
Alignment is a little funny here. Since there's only one item in the object, can you put that all on … |
david | |
Doc comment? |
david | |
Doc comments throughout? |
david | |
Can you use $.param to format the querystring args? |
david | |
Can you assert that instance is null? |
david | |
You can flip this around and default options into your new object, which makes it so you can skip the … |
david | |
Since this is global, instead of assigning to var, we probably should be doing window.SUPPORT_DATA = .... |
david | |
Alphabetical order. |
chipx86 | |
Maybe I'm thinking a different language, but I thought you had to wrap new Blah in parens if you wanted … |
chipx86 | |
Rather than doing this and building the script yourself, you can just do: $.ajax({ url: RB.SupportBannerView.supportURL, data: { 'support-data': SUPPORT_DATA, … |
chipx86 | |
This isn't a boolean. If we want to check it against an empty string, we should do that explicitly. |
chipx86 | |
You can just do var SUPPORT_DATA, rather than explicitly attaching it to window. |
chipx86 | |
Typo. = |
chipx86 | |
Is the alignment a bit off here? Shouldn't it be 2 spaces in, not 4? |
imadueme | |
Is the alignment a bit off here? Shouldn't it be 2 spaces in, not 4? |
imadueme | |
Doesn't look like you added this, but, are vim preferences supposed to be in here? |
imadueme | |
Blank line in between these. |
david |
-
Can you move the admin.js refactor into a separate change?
It also looks like
backbone-1.0.0.min.js
is no longer a minified file. What happened there? -
-
-
Alignment is a little funny here. Since there's only one item in the object, can you put that all on one line?
-
-
-
-
-
You can flip this around and default
options
into your new object, which makes it so you can skip the|| {}
part. -
Since this is global, instead of assigning to var, we probably should be doing
window.SUPPORT_DATA = ...
.
- Description:
-
The support banner shows the current support status, via
beanbaginc.com's API. While the data is being retrieved, a spinner is being shown. - - This patch also reorganizes all the admin JavaScript into the
- js/admin/
directory.
-
Tool: PEP8 Style Checker Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js Tool: Pyflakes Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js
- Diff:
-
Revision 3 (+122 -7)
-
Tool: Pyflakes Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/webhookFormView.js reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/js/admin/repositoryform.js Tool: PEP8 Style Checker Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/webhookFormView.js reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/js/admin/repositoryform.js
-
Tool: Pyflakes Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js Tool: PEP8 Style Checker Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js
-
-
-
Maybe I'm thinking a different language, but I thought you had to wrap
new Blah
in parens if you wanted to immediately call a function on it?I think I'd prefer the standard thing of two statements: One instantiating, one rendering. Easier to debug if things go wrong, too.
-
Rather than doing this and building the script yourself, you can just do:
$.ajax({ url: RB.SupportBannerView.supportURL, data: { 'support-data': SUPPORT_DATA, }, dataType: 'jsonp', jsonpCallback: 'RB.SupportBannerView.instance.receive' });
That will handle all the work of doing the JSONP stuff, along with handling cache busting.
-
-
-
- Diff:
-
Revision 5 (+202 -8)
- Removed Files:
- Added Files:
-
Tool: Pyflakes Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/css/ui/boxes.less reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less Tool: PEP8 Style Checker Processed Files: reviewboard/admin/views.py reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/admin/admin.js reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/css/ui/boxes.less reviewboard/templates/admin/dashboard.html reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less reviewboard/static/rb/css/ui/boxes.less Tool: Pyflakes Ignored Files: reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less reviewboard/static/rb/css/ui/boxes.less
-
Tool: Pyflakes Ignored Files: reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less reviewboard/static/rb/css/ui/boxes.less Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/css/pages/admin-dashboard.less reviewboard/static/rb/js/admin/views/supportBannerView.js reviewboard/static/rb/css/defs.less reviewboard/static/rb/css/ui/boxes.less