Adding description field to features.add() #1881

sunilkumarc · 2019-03-23T19:11:23Z

@sindresorhus I've now added description field to features.add() and also description text for all of the existing features. Please review and let me know if any changes are required.

Fixes #1787

source/features/add-co-authored-by.tsx

source/features/comment-fields-keyboard-shortcuts.tsx

source/features/commented-menu-item.tsx

notlmn · 2019-03-24T04:02:58Z

source/features/copy-file.tsx

@@ -30,6 +30,7 @@ function init() {

 features.add({
 	id: 'copy-file',
+	description: 'Copy file',


Suggested change

description: 'Copy file',

description: 'Adds button to copy file contents to clipboard',

source/features/copy-on-y.tsx

source/features/create-release-shortcut.tsx

source/features/download-folder-button.tsx

source/features/embed-gist-inline.tsx

source/features/linkify-commit-sha.tsx

source/features/reactions-avatars.tsx

source/features/show-user-top-repositories.tsx

notlmn · 2019-03-24T04:14:18Z

source/features/warn-pr-from-master.tsx

@@ -27,6 +27,7 @@ async function init() {

 features.add({
 	id: 'warn-pr-from-master',
+	description: 'Show warning when a user attempts to create a PR from their fork\'s default branch',


Same as https://github.com/sindresorhus/refined-github/pull/1881/files#r268417701

notlmn · 2019-03-24T04:19:45Z

Provided some suggestions.

But still confused on how the description should sound like, like 'Adds keyboard shortcuts to comment fields' or just leaving it as 'Commented menu item' which would otherwise be ambiguous. Or if the words Adds should be added to all (most) of the features descriptions.

Looking out for what @bfred-it thinks about these.

fregante · 2019-03-24T05:17:17Z

Some files already have a description and sometimes links in a comment in the JS file header. These can be removed from there and used as-is in the description field.

If they are multiline, they can stay as:

features.add({
	id: 'reactions-avatars',
	description: `
		Line 1
		Line 2
		https://
	`

Eventually we might have a mini-parser for links and backticks (like in #1872 )

notlmn · 2019-03-24T05:44:33Z

Some files already have a description and sometimes links in a comment in the JS file header. These can be removed from there and used as-is in the description field.

Yup. And highly agree for multi-line descriptions!

Eventually we might have a mini-parser for links and backticks (like in #1872)

Thinking if the description field can be used to render a checkbox-style list in the extensions options to enable/disable individual features. 🤷‍♂️

fregante · 2019-03-24T05:53:30Z

Thinking if the description field can be used to render a checkbox-style list in the extensions options to enable/disable individual features. 🤷‍♂️

That's the idea, when I finish #1780

notlmn · 2019-03-24T05:59:14Z

That's the idea, when I finish #1780

Hell, I missed that PR! 😅

sindresorhus · 2019-03-24T11:38:59Z

The description field should be required, meaning the code should ensure it exists.

sindresorhus · 2019-03-24T11:40:52Z

Many of the descriptions are too short or ambiguous. Can you put some more time into writing good descriptions? :)

… into fix-1787

sunilkumarc · 2019-03-27T19:13:35Z

Although this code base is completely new to me, I've done my best to add meaningful description to most of the features. Still I would need your help in filling description for below features:

upload-button
toggle-files-button
scroll-to-top-on-collapse
pull-request-hotkey
more-dropdown
hide-empty-meta
close-out-of-view-modals
ci-link
global-discussion-list-filters

@sindresorhus I think the description field is already a required field. Without adding this field, calling features.add() would fail saying Property 'description' is missing in type. Is this not the right way? If not, please let me know how I can ensure description field in features.

sindresorhus · 2019-03-27T20:07:14Z

Still I would need your help in filling description for below features:

Go to the feature's JS file, click the History button, find the first commit, it should show you the readme addition which describes the feature. For example, this is the commit for ci-link: 457071f#diff-19f501127e11d5e0cc6f4c78bc44bc57

You will need the https://chrome.google.com/webstore/detail/follow-for-github/agalokjhnhheienloigiaoohgmjdpned?hl=en extension, so you can find the first commit, even if it was renamed.

sindresorhus · 2019-03-27T20:08:55Z

I think the description field is already a required field. Without adding this field, calling features.add() would fail saying Property 'description' is missing in type. Is this not the right way? If not, please let me know how I can ensure description field in features.

Yup, I forgot we can enforce this with types now.

fregante · 2019-03-28T06:22:03Z

You will need the chrome.google.com/webstore/detail/follow-for-github/agalokjhnhheienloigiaoohgmjdpned?hl=en extension, so you can find the first commit, even if it was renamed.

I just merged this feature in Refined GitHub. You'll just need the latest version from the extensions store.

… into fix-1787

sunilkumarc · 2019-03-28T18:58:58Z

@notlmn @bfred-it @sindresorhus I've added the description texts by following above suggested approaches. Please review and let me know if something needs to be changed.

notlmn · 2019-03-31T04:37:51Z

source/features/emojis-title.tsx

@@ -12,6 +12,7 @@ function init() {

 features.add({
 	id: 'emojis-title',
+	description: 'Don’t add a title if the emoji\'s parents already have one',


Styled quote (’) and escaped quote (\')?

Choose to have only one of these (throughout the other files too).

I would prefer \'.

I read this description many times and I'm still not sure what the feature actually does.

Sidetracking here, but it takes the alias attribute (provided by GH) of the emoji, and adds it as a title (if any of its closest parent doesn't provide one).

Presumably so that anyone without access to an emoji keyboard (picket) can use this info to insert emojis in comments.

Considering that GitHub already provides autocomplete for emojis in desktop view and that the title attribute is useless on mobile devices, I think this feature can be dropped. 🤷‍♂️

Context:
#278
#437

I don't think I ever used it and even then you can just copy-paste the emoji.

I would prefer \'.

@bfred-it wants ’, so we'll go with that. But should be consistent everywhere.

notlmn · 2019-03-31T04:39:49Z

source/features/filter-comments-by-you.tsx

@@ -21,6 +21,7 @@ function init() {

 features.add({
 	id: 'filter-comments-by-you',
+	description: 'Search for issues and PRs with the `Everything commented by you filter`',


@bfred-it Would this look good after backticks are parsed?

It's not exactly code, per se. Or can bold be used in cases like these?

It's how we've been marking them up in the readme: https://github.com/sindresorhus/refined-github/blob/0b6c7330c21f8570519143ad3d0decc328d4db47/readme.md#L185

@sindresorhus thoughts? Should they be in italics (in the readme too)?

Edit: wow, ironically the link you can only generate thanks to RGH (#1873) is correctly embedded by GitHub

@sindresorhus any comments on this? I would like to close this PR as soon as possible.

Yes, let's go with italic.

sunilkumarc · 2019-04-02T17:43:13Z

@bfred-it Any comments from you for latest changes? I'll work on the changes suggested by @notlmn along with your suggestions, if there are any.

sindresorhus · 2019-04-17T14:35:10Z

Can you fix the merge conflict?

sindresorhus · 2019-04-17T14:36:08Z

source/features/ci-link.tsx

@@ -35,6 +35,7 @@ async function init() {

 features.add({
 	id: 'ci-link',
+	description: 'Adds build status and link to CI by the repo\'s title',


Be consistent with Add and Adds. I prefer Add.

sindresorhus · 2019-04-17T14:36:41Z

source/features/comment-fields-keyboard-shortcuts.tsx

@@ -81,6 +81,7 @@ function init() {

 features.add({
 	id: 'comment-fields-keyboard-shortcuts',
+	description: 'Quickly edit your last comment using the `↑` shortcut',


Suggested change

description: 'Quickly edit your last comment using the `↑` shortcut',

description: 'Quickly edit your last comment using the `↑` keyboard shortcut',

sindresorhus · 2019-04-17T14:37:31Z

source/features/copy-on-y.tsx

@@ -19,6 +19,7 @@ function deinit() {

 features.add({
 	id: 'copy-on-y',
+	description: 'Adds shortcut to copy permalink of file to clipboard',


Suggested change

description: 'Adds shortcut to copy permalink of file to clipboard',

description: 'Adds shortcut to copy the permalink of a file to the clipboard',

sindresorhus · 2019-04-17T14:38:09Z

source/features/download-folder-button.tsx

@@ -17,6 +17,7 @@ function init() {

 features.add({
 	id: 'download-folder-button',
+	description: 'Download entire folders from repositories using the `Download folder` button. (Uses download-directory.github.io)',


Suggested change

description: 'Download entire folders from repositories using the `Download folder` button. (Uses download-directory.github.io)',

description: 'Download entire folders from repositories using the `Download folder` button. (Uses https://download-directory.github.io)',

sindresorhus · 2019-04-17T14:38:54Z

source/features/extend-diff-expander.tsx

@@ -15,6 +15,7 @@ function init() {

 features.add({
 	id: 'extend-diff-expander',
+	description: 'Widen `Expand diff` button',


Suggested change

description: 'Widen `Expand diff` button',

description: 'Widen the `Expand diff` button',

sindresorhus · 2019-04-17T14:39:20Z

source/features/extend-status-labels.tsx

@@ -30,6 +30,7 @@ function init() {

 features.add({
 	id: 'extend-status-labels',
+	description: 'Extend status labels',


Extend them with what or in what way? The description is not clear enough.

sindresorhus · 2019-04-17T14:56:11Z

source/features/user-profile-follower-badge.tsx

@@ -19,6 +19,7 @@ async function init() {

 features.add({
 	id: 'user-profile-follower-badge',
+	description: 'Badge on user\'s profile if they follow you',


Suggested change

description: 'Badge on user\'s profile if they follow you',

description: 'Add a badge on user\'s profile page if they follow you',

sindresorhus · 2019-04-17T14:56:31Z

source/features/view-markdown-source.tsx

@@ -89,6 +85,9 @@ async function init(): Promise<false | void> {

 features.add({
 	id: 'view-markdown-source',
+	description: `Add button to view the markdown source whereas GitHub only lets you see the rendered version.


Suggested change

description: `Add button to view the markdown source whereas GitHub only lets you see the rendered version.

description: `Add a button to view the Markdown source whereas GitHub only lets you see the rendered version.

sindresorhus · 2019-04-17T14:56:49Z

source/features/wait-for-build.tsx

@@ -115,6 +115,7 @@ function init() {

 features.add({
 	id: 'wait-for-build',
+	description: 'The option to wait for checks when merging a PR',


Suggested change

description: 'The option to wait for checks when merging a PR',

description: 'Add checkbox to wait for checks when merging a PR',

sindresorhus · 2019-04-17T14:57:02Z

source/features/warn-pr-from-master.tsx

@@ -27,6 +19,11 @@ async function init() {

 features.add({
 	id: 'warn-pr-from-master',
+	description: `Creating a PR from the master branch is an anti-pattern. This feature produces a
+			warning when a user attempts to create a PR from their fork's default branch.


Don't hard-wrap.

sindresorhus · 2019-04-17T14:57:25Z

source/features/warning-for-disallow-edits.tsx

@@ -31,6 +31,7 @@ function init() {

 features.add({
 	id: 'warning-for-disallow-edits',
+	description: 'A warning appears when unchecking `Allow edits from maintainers`',


Suggested change

description: 'A warning appears when unchecking `Allow edits from maintainers`',

description: 'Add warning when unchecking `Allow edits from maintainers`',

fregante · 2019-04-24T04:02:11Z

Ping on this issue, if possible we'd like to merge it soon

fregante · 2019-04-27T14:36:08Z

If you can at least merge the suggestions by @sindresorhus we can accept it, otherwise we have to close it

1. Open refined-github#1881 2. Expand add-co-authored-by.tsx 3. Notice no indentation change in the "Suggested change" diff

Adding description field to features.add()

a5203ea