Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add lock for private orgs on own profile #1490

Merged
merged 14 commits into from
Sep 11, 2018
Merged

Add lock for private orgs on own profile #1490

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
52f05fc
Add lock for private orgs on own profile
aramperes Aug 26, 2018
08e0407
Fix lint issues
aramperes Aug 26, 2018
b29a2c1
Improve looks
aramperes Aug 26, 2018
86a9663
Add outline to lock
aramperes Aug 26, 2018
3007104
Add readme entry
aramperes Aug 26, 2018
d749fca
Address some review comments
aramperes Aug 27, 2018
5f1e740
Lint
aramperes Aug 27, 2018
565d858
Proper SVG resizing, readme adjustment
aramperes Aug 27, 2018
15eed7a
Fix tests
aramperes Aug 27, 2018
d8d60ba
Properly fix tests, revert workaround
aramperes Aug 27, 2018
6c52679
Extra line
aramperes Aug 27, 2018
c4f5e56
Add some more mock globals
aramperes Aug 27, 2018
b979f6e
Merge branch 'master' into mark-private-orgs
sindresorhus Sep 11, 2018
29453ae
Update mark-private-orgs.js
sindresorhus Sep 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion source/features/upload-button.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function triggerUploadUI({target}) {
}

function handleKeydown(event) {
if (event[metaKey] && event.key === 'u') {
if (event[metaKey()] && event.key === 'u') {
triggerUploadUI(event);
event.preventDefault();
}
Expand Down
4 changes: 2 additions & 2 deletions source/libs/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,9 @@ export const flatZip = (table, limit = Infinity) => {
return zipped;
};

export const isMac = /Mac/.test(window.navigator.platform);
export const isMac = () => /Mac/.test(window.navigator.platform);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer not having to change code because of testing. I already commented a way to fix this properly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I said, the issue itself isn't because of a missing mock for the platform property, but rather that the window global isn't initialized when the utils file is imported.

import test from 'ava';
import * as pageDetect from '../source/libs/page-detect'; // Imports utils.js, error is thrown because no window global
import Window from './fixtures/window';

// global mocks
global.window = new Window();
global.location = window.location;
global.document = {};

I tried creating a new file which sets the window global mocks, and is imported before the page-detect file. That didn't appear to solve the issue, as the utils file is being imported from somewhere else (?)

Copy link
Contributor Author

@aramperes aramperes Aug 27, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests should be fixed properly now, and I reverted the workaround.

e: looks like some more things need to be mocked


export const metaKey = isMac ? 'metaKey' : 'ctrlKey';
export const metaKey = () => isMac() ? 'metaKey' : 'ctrlKey';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not affecting the tests.


export const anySelector = selector => {
const prefix = document.head.style.MozOrient === '' ? 'moz' : 'webkit';
Expand Down