Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(3773)

Issue 7299071: Add a warning about unsupported browsers.

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by benji
Modified:
11 years, 1 month ago
Reviewers:
mp+147448
Visibility:
Public.

Description

Add a warning about unsupported browsers. The tests for the code are a bit unusual in that the code under test is embedded in an HTML file, so it must be extracted and the test file built my the Makefile so it can be tested. I don't think this approach is too heinous. https://code.launchpad.net/~benji/juju-gui/bug-1104105-browser-compatability-warning-3/+merge/147448 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Add a warning about unsupported browsers. #

Total comments: 14

Patch Set 3 : Add a warning about unsupported browsers. #

Total comments: 11

Patch Set 4 : Add a warning about unsupported browsers. #

Patch Set 5 : Add a warning about unsupported browsers. #

Patch Set 6 : Add a warning about unsupported browsers. #

Patch Set 7 : Add a warning about unsupported browsers. #

Patch Set 8 : Add a warning about unsupported browsers. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -5 lines) Patch
A [revision details] View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M lib/views/stylesheet.less View 1 2 3 4 5 2 chunks +5 lines, -5 lines 0 comments Download

Messages

Total messages: 10
benji
Please take a look.
11 years, 1 month ago (2013-02-08 18:57:20 UTC) #1
benji
Please take a look.
11 years, 1 month ago (2013-02-08 19:47:20 UTC) #2
bac
The branch looks good, thanks Benji. One change to make before landing. https://codereview.appspot.com/7299071/diff/3001/app/index.html File app/index.html ...
11 years, 1 month ago (2013-02-10 21:44:58 UTC) #3
gary.poster
Hi Benji. Interesting test approach that is cool. Will think about it more. I raised ...
11 years, 1 month ago (2013-02-11 13:32:19 UTC) #4
benji
Please take a look. https://codereview.appspot.com/7299071/diff/3001/Makefile File Makefile (right): https://codereview.appspot.com/7299071/diff/3001/Makefile#newcode523 Makefile:523: server spritegen test test-debug test-prod ...
11 years, 1 month ago (2013-02-11 17:52:51 UTC) #5
benji
Thanks guys for the good reviews. The branch has been updated with fixes for everything ...
11 years, 1 month ago (2013-02-11 17:54:10 UTC) #6
gary.poster
Land with changes, unless you argue against the changes. Cool, thank you, and thanks for ...
11 years, 1 month ago (2013-02-11 19:08:03 UTC) #7
benji
Submitting. https://codereview.appspot.com/7299071/diff/3002/app/index.html File app/index.html (right): https://codereview.appspot.com/7299071/diff/3002/app/index.html#newcode108 app/index.html:108: <script id="app-startup"> On 2013/02/11 19:08:03, gary.poster wrote: > ...
11 years, 1 month ago (2013-02-11 20:18:23 UTC) #8
benji
*** Submitted: Add a warning about unsupported browsers. The tests for the code are a ...
11 years, 1 month ago (2013-02-11 20:22:00 UTC) #9
benji
11 years, 1 month ago (2013-02-11 22:10:22 UTC) #10
*** Submitted:

Add a warning about unsupported browsers.

The tests for the code are a bit unusual in that the code under test is
embedded in an HTML file, so it must be extracted and the test file built my
the Makefile so it can be tested.  I don't think this approach is too heinous.

R=
CC=
https://codereview.appspot.com/7299071
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b