Fix: Preview error viewer in platform and IE11 #696
Conversation
tonyjin
commented
Mar 2, 2018
tonyjin
commented
- Contact Us redirect to Box support page should only show up in the Box web application
- Fix non-wrapping error text in IE11
- Consolidate download checks into util function
- Move file-specific util method into file.js
|
Verified that @tonyjin has signed the CLA. Thanks for the pull request! |
src/lib/util.js
Outdated
| * @return {boolean} Is Preview running in the Box WebApp | ||
| */ | ||
| export function inBoxWebApp() { | ||
| return (window.location.hostname || '').indexOf('app.box.com') !== -1; |
There was a problem hiding this comment.
We have an 'includes' polyfill if you want to use that
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/includes
There was a problem hiding this comment.
Actually I'm gonna hold off adding a polyfill for now. The existing polyfill is for Array.prototype.includes. If I included the String polyfill, I'd want to update all references of indexOf and that looks like quite a few files.
There was a problem hiding this comment.
Seems like we follow an isSomething format for boolean returning functions. Maybe isBoxWebApp, or the inverse function isPlatformApp
src/lib/__tests__/util-test.js
Outdated
| expect(util.inBoxWebApp()).to.be.false; | ||
| }); | ||
|
|
||
| /* No quick way to stub window.location.hostname to test for truth case */ |
There was a problem hiding this comment.
According to SO this works for href, maybe will work for hostname too?
global.window = {
location: {
href: {
value: 'foo'
}
}
}
There was a problem hiding this comment.
I found that thread and wasted ~an hour yesterday trying to get the test to work unsuccessfully. Tried a few things including Object.defineProperty, global.window (which wouldn't work in the browser anyways I think), and a few others.
tonyjin
force-pushed
the
non-downloadable-contact-us
branch
from
0289230
to
3d1d105
Compare
src/lib/util.js
Outdated
| * @return {boolean} Is Preview running in the Box WebApp | ||
| */ | ||
| export function inBoxWebApp() { | ||
| return (window.location.hostname || '').indexOf('app.box.com') !== -1; |
There was a problem hiding this comment.
Seems like we follow an isSomething format for boolean returning functions. Maybe isBoxWebApp, or the inverse function isPlatformApp
src/lib/__tests__/Preview-test.js
Outdated
|
|
||
| it('should show not print button if print is not supported', () => { |
src/lib/__tests__/Preview-test.js
Outdated
| it('should show print button if print is supported', () => { | ||
| stubs.checkFeature.returns(false); | ||
|
|
||
| it('should show not print button if file can\'t be downloaded', () => { |
- Contact Us redirect to Box support page should only show up in the Box web application - Fix non-wrapping error text in IE11 - Consolidate download checks into util function - Move file-specific util method into file.js
tonyjin
force-pushed
the
non-downloadable-contact-us
branch
from
3d1d105
to
09f06a8
Compare
