Add lock for private orgs on own profile #1490

aramperes · 2018-08-26T05:56:36Z

Adds a little "lock" octicon for private organizations on a user's own profile.

UI is kinda ew, suggestions welcome!

Example:

waldyrious · 2018-08-26T18:00:42Z

I think just making the inner part of the lock non-transparent (e.g. white or very light grey) would make this quite visually acceptable :)

aramperes · 2018-08-26T18:45:32Z

@waldyrious Thanks for the suggestion!

New look:

waldyrious · 2018-08-26T21:05:40Z

Looks much better, nice work! I think a small detail that could improve the visibility further is a thin white outline so that the shape of the lock is not lost in darker backgrounds. I would also not fill in the inside of the lock's arc (but would add the outline there too).

waldyrious · 2018-08-26T21:16:58Z

Here's a suggestion:

<svg xmlns="http://www.w3.org/2000/svg" height="16.426" width="13.426" viewBox="-118 118 13.426 16.426">
  <path d="M-111.287 118c-2.59 0-4.713 2.124-4.713 4.713V124h-.287c-.488 0-.892.222-1.192.521-.3.3-.521.704-.521 1.192v7c0 .488.222.892.521 1.191.3.3.704.522 1.192.522h10c.488 0 .892-.222 1.191-.522.3-.3.522-.703.522-1.191v-7c0-.488-.222-.892-.522-1.192-.3-.3-.703-.521-1.191-.521h-.287v-1.287c0-2.59-2.124-4.713-4.713-4.713zm0 3.227c.804 0 1.486.682 1.486 1.486V124h-2.972v-1.287c0-.804.682-1.486 1.486-1.486z" fill="#fff"/>
  <path d="M-106.287 124.713h-1v-2c0-2.2-1.8-4-4-4s-4 1.8-4 4v2h-1c-.5 0-1 .5-1 1v7c0 .5.5 1 1 1h10c.5 0 1-.5 1-1v-7c0-.5-.5-1-1-1zm-7.2-2c0-1.2 1-2.2 2.2-2.2 1.2 0 2.2 1 2.2 2.2v2h-4.4z" fill="#333"/>
  <path d="M-115.287 132.713h9v-7h-9v7zm1-6h1v1h-1v-1zm0 2h1v1h-1v-1zm0 2h1v1h-1v-1z" fill="#eee"/>
  <path fill="#333" d="M-114.287 130.713h1v1h-1zM-114.287 126.713h1v1h-1zM-114.287 128.713h1v1h-1z"/>
</svg>

I've uploaded it to svgshare, which conveniently shows how it looks on a variety of backgrounds:

aramperes · 2018-08-26T22:00:01Z

Thanks, update:

sindresorhus · 2018-08-27T04:56:37Z

source/features/mark-private-orgs.js

+import api from '../libs/api';
+
+export default async () => {
+	if (!getCleanPathname().startsWith(getUsername())) {


I think this should be added to https://github.com/sindresorhus/refined-github/blob/30071048dd10a1b12ade945c198951e262199631/source/libs/page-detect.js as isOwnUserProfile

sindresorhus · 2018-08-27T04:57:02Z

readme.md

@@ -135,6 +135,7 @@ GitHub Enterprise is also supported. More info in the options.
 - [Review counts are shown and colored in PR lists.](https://user-images.githubusercontent.com/1402241/33474535-a814ee78-d6ad-11e7-8f08-a8b72799e376.png)
 - [All available keyboard shortcuts are shown in the help modal.](https://user-images.githubusercontent.com/29176678/36999174-9f07d33e-20bf-11e8-83e3-b3a9908a4b5f.png)  *(<kbd>?</kbd> hotkey)*
 - [Followers you know are shown on profile pages.](https://user-images.githubusercontent.com/2906365/42009293-b1503f62-7a57-11e8-88f5-9c2fb3651a14.png)
+- [Private organizations are marked on your profile.](https://user-images.githubusercontent.com/6775216/44633467-d5dcc900-a959-11e8-9116-e6b0ffef66af.png)


Should be clear this is only visible when you are viewing your profile.

sindresorhus · 2018-08-27T05:08:02Z

source/features/mark-private-orgs.js

+	}
+	// List public orgs
+	const publicOrgs = [];
+	const orgDataList = await api(`users/${getUsername()}/orgs`, true);


It would be better to get the information whether the org is private or not from the JSON payload, rather than doing an anonymous request. And authorized request has other benefits, like much higher rate limit, so we should prefer that whenever possible.

Turns out this endpoint returns the public orgs, whether or not the user is signed in. No need for anon requests.

sindresorhus · 2018-08-27T05:10:10Z

source/libs/api.js

@@ -2,7 +2,7 @@ import OptionsSync from 'webext-options-sync';

 const cache = new Map();

-export default async endpoint => {
+export default async (endpoint, anonymousRequest = false) => {


Use an options-object instead of a boolean argument.

sindresorhus · 2018-08-27T05:10:41Z

source/features/mark-private-orgs.js

+		publicOrgs.push('/' + orgData.login);
+	}
+
+	// Display


This comment is not useful. Display doesn't mean anything without any context.

sindresorhus · 2018-08-27T05:11:35Z

source/features/mark-private-orgs.js

+import * as icons from '../libs/icons';
+import api from '../libs/api';
+
+export default async () => {


Use a normal non-arrow async function for consistency with other features.

sindresorhus · 2018-08-27T05:13:08Z

source/libs/icons.js

+
+export const privateLock = () => <svg className="octicon octicon-lock" viewBox="0 0 12 16" version="1.1" width="12" height="16" aria-hidden="true"><path fill-rule="evenodd" d="M4 13H3v-1h1v1zm8-6v7c0 .55-.45 1-1 1H1c-.55 0-1-.45-1-1V7c0-.55.45-1 1-1h1V4c0-2.2 1.8-4 4-4s4 1.8 4 4v2h1c.55 0 1 .45 1 1zM3.8 6h4.41V4c0-1.22-.98-2.2-2.2-2.2-1.22 0-2.2.98-2.2 2.2v2H3.8zM11 7H2v7h9V7zM4 8H3v1h1V8zm0 2H3v1h1v-1z" /></svg>;
+
+export const privateLockFilled = () =>


Would be better to accept a size argument here instead of using CSS transform.

aramperes · 2018-08-27T05:35:08Z

A test is failing because of the getUsername call in the page_detect.js file. I'm not familiar with the testing environment of this project, could someone point me in the right direction?

sindresorhus · 2018-08-27T05:42:40Z

source/libs/icons.js

-export const privateLockFilled = () =>
-	<svg version="1.1" className="octicon octicon-lock" height="16.426" width="13.426" viewBox="-118 118 13.426 16.426">
+export const privateLockFilled = (width, height) =>
+	<svg version="1.1" className="octicon octicon-lock" height={height} width={width} viewBox={`-118 118 ${width} ${height}`}>


This is not correct. viewBox should not be touched.

And it should be a size argument with one value that is used for both width/height. The viewBox should ensure to keep the aspect ratio.

sindresorhus · 2018-08-27T05:45:25Z

readme.md

@@ -135,6 +135,7 @@ GitHub Enterprise is also supported. More info in the options.
 - [Review counts are shown and colored in PR lists.](https://user-images.githubusercontent.com/1402241/33474535-a814ee78-d6ad-11e7-8f08-a8b72799e376.png)
 - [All available keyboard shortcuts are shown in the help modal.](https://user-images.githubusercontent.com/29176678/36999174-9f07d33e-20bf-11e8-83e3-b3a9908a4b5f.png)  *(<kbd>?</kbd> hotkey)*
 - [Followers you know are shown on profile pages.](https://user-images.githubusercontent.com/2906365/42009293-b1503f62-7a57-11e8-88f5-9c2fb3651a14.png)
+- [Private organizations are marked on your own profile.](https://user-images.githubusercontent.com/6775216/44633467-d5dcc900-a959-11e8-9116-e6b0ffef66af.png)


Private organizations are marked when viewing your own profile.

sindresorhus · 2018-08-27T05:48:08Z

getUsername depends on the DOM, but the tests for page-detect does not use the DOM. You need to mock getUsername.

aramperes · 2018-08-27T06:02:55Z

Actually, the error appears to come from the import statement:

import {getUsername} from './utils';

/home/momo/dev/refined-github/source/libs/utils.js:173
const isMac = exports.isMac = /Mac/.test(window.navigator.platform);
                                         ^

ReferenceError: window is not defined

Could the isMac/metaKey constants simply be transformed to functions?

sindresorhus · 2018-08-27T06:10:16Z

I think you can just mock window.navigator.platform here: https://github.com/sindresorhus/refined-github/blob/master/test/fixtures/window.js

aramperes · 2018-08-27T06:43:50Z

The window mock global is set after the import statement, it needs to be initialized before the utils.js file is initialized anywhere. I used the isMac/metaKey fix as a workaround.

sindresorhus · 2018-08-27T06:44:27Z

source/libs/utils.js


-export const metaKey = isMac ? 'metaKey' : 'ctrlKey';
+export const metaKey = () => isMac() ? 'metaKey' : 'ctrlKey';


This is not affecting the tests.

sindresorhus · 2018-08-27T06:44:49Z

source/libs/utils.js

@@ -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);


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

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 (?)

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

e: looks like some more things need to be mocked

test/searchable-words.js

sindresorhus · 2018-09-11T05:31:18Z

This is looking good now. Thanks, @Momothereal :)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

sfdye · 2018-09-11T06:12:57Z

Now that this is merged to master, when can we receive the update from Chrome store? I saw there is .travis.yml file with a deploy section but didn't see tags being used in this repo?

sindresorhus · 2018-09-11T06:18:52Z

A new release is automatically published every 24 hours (if there are new commits).

https://github.com/sindresorhus/refined-github/blob/bebc684d32a5c68647fac57d1251e0da0c627f3b/package.json#L14

sfdye · 2018-09-11T06:19:37Z

@sindresorhus Aha got it. Thanks!

aramperes added 2 commits August 26, 2018 01:51

Add lock for private orgs on own profile

52f05fc

Fix lint issues

08e0407

Improve looks

b29a2c1

Add outline to lock

86a9663

Add readme entry

3007104

sindresorhus requested changes Aug 27, 2018

View reviewed changes

aramperes added 2 commits August 27, 2018 01:28

Address some review comments

d749fca

Lint

5f1e740

sindresorhus reviewed Aug 27, 2018

View reviewed changes

Proper SVG resizing, readme adjustment

565d858

Fix tests

15eed7a

sindresorhus reviewed Aug 27, 2018

View reviewed changes

aramperes added 3 commits August 27, 2018 12:18

Properly fix tests, revert workaround

d8d60ba

Extra line

6c52679

Add some more mock globals

c4f5e56

fregante reviewed Sep 5, 2018

View reviewed changes

test/searchable-words.js Show resolved Hide resolved

sindresorhus added 2 commits September 11, 2018 12:18

Merge branch 'master' into mark-private-orgs

b979f6e

Update mark-private-orgs.js

29453ae

sindresorhus merged commit 4b10087 into refined-github:master Sep 11, 2018

sindresorhus added a commit that referenced this pull request Sep 11, 2018

Add lock for private orgs on own profile (#1490)

48d58d7

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add lock for private orgs on own profile #1490

Add lock for private orgs on own profile #1490

aramperes commented Aug 26, 2018 •

edited

waldyrious commented Aug 26, 2018

aramperes commented Aug 26, 2018

waldyrious commented Aug 26, 2018

waldyrious commented Aug 26, 2018

aramperes commented Aug 26, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

aramperes Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

aramperes commented Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus commented Aug 27, 2018

aramperes commented Aug 27, 2018 •

edited

sindresorhus commented Aug 27, 2018

aramperes commented Aug 27, 2018

sindresorhus Aug 27, 2018

sindresorhus Aug 27, 2018

aramperes Aug 27, 2018

aramperes Aug 27, 2018 •

edited

sindresorhus commented Sep 11, 2018

sfdye commented Sep 11, 2018

sindresorhus commented Sep 11, 2018

sfdye commented Sep 11, 2018


		export const privateLock = () => <svg className="octicon octicon-lock" viewBox="0 0 12 16" version="1.1" width="12" height="16" aria-hidden="true"><path fill-rule="evenodd" d="M4 13H3v-1h1v1zm8-6v7c0 .55-.45 1-1 1H1c-.55 0-1-.45-1-1V7c0-.55.45-1 1-1h1V4c0-2.2 1.8-4 4-4s4 1.8 4 4v2h1c.55 0 1 .45 1 1zM3.8 6h4.41V4c0-1.22-.98-2.2-2.2-2.2-1.22 0-2.2.98-2.2 2.2v2H3.8zM11 7H2v7h9V7zM4 8H3v1h1V8zm0 2H3v1h1v-1z" /></svg>;

		export const privateLockFilled = () =>


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

Add lock for private orgs on own profile #1490

Add lock for private orgs on own profile #1490

Conversation

aramperes commented Aug 26, 2018 • edited

waldyrious commented Aug 26, 2018

aramperes commented Aug 26, 2018

waldyrious commented Aug 26, 2018

waldyrious commented Aug 26, 2018

aramperes commented Aug 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aramperes commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sindresorhus commented Aug 27, 2018

aramperes commented Aug 27, 2018 • edited

sindresorhus commented Aug 27, 2018

aramperes commented Aug 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aramperes Aug 27, 2018 • edited

Choose a reason for hiding this comment

sindresorhus commented Sep 11, 2018

sfdye commented Sep 11, 2018

sindresorhus commented Sep 11, 2018

sfdye commented Sep 11, 2018

aramperes commented Aug 26, 2018 •

edited

aramperes commented Aug 27, 2018 •

edited

aramperes Aug 27, 2018 •

edited