Add isUserSettings
#73
Conversation
index.ts
Outdated
| @@ -377,6 +377,11 @@ collect.set('isRepoMainSettings', [ | |||
| 'https://github.com/sindresorhus/refined-github/settings', | |||
| ]); | |||
|
|
|||
| export const isUserSettings = (url: URL | HTMLAnchorElement | Location = location): boolean => getCleanPathname(url) === 'settings'; | |||
| collect.set('isUserSettings', [ | |||
| 'https://github.com/settings', | |||
There was a problem hiding this comment.
This page doesn't exist, it's a redirect to https://github.com/settings/profile and this page doesn't match isUserSettings.
I think the check should be url.pathname.startsWith('/settings/')
There was a problem hiding this comment.
I think this is why we never made that. We always made specific pages as we needed it (I did not notice the redirect as I was on mobile)
There was a problem hiding this comment.
What do you need this for @stefanbuck?
There was a problem hiding this comment.
Good shout. Addressed in 1bf7bdf
There was a problem hiding this comment.
@stefanbuck what is your use case?
There was a problem hiding this comment.
@stefanbuck what is your use case?
I'm working on a new browser extension and I was thinking about adding the extension settings right into GitHub rather than using an extension settings dialog.
There was a problem hiding this comment.
Looking forward to seeing that! Are you planning to use webext-options-sync?
There was a problem hiding this comment.
webext-options-sync looks neat, so I will consider using it.
index.ts
Outdated
| @@ -377,6 +377,12 @@ collect.set('isRepoMainSettings', [ | |||
| 'https://github.com/sindresorhus/refined-github/settings', | |||
| ]); | |||
|
|
|||
| export const isUserSettings = (url: URL | HTMLAnchorElement | Location = location): boolean => url.pathname.startsWith('/settings/'); | |||
| collect.set('isUserSettings', [ | |||
| 'https://github.com/settings/', | |||
There was a problem hiding this comment.
This doesn't exist and should be removed.
Also the tests are failing because all the URLs in this file are tested against each other, so if any other URL matches this function, it should be included in this array. The test error will tell you exactly which URLs are missing.
There was a problem hiding this comment.
Sorry my fault, I was in rush and only made the change without running the tests locally first. I made the change in 229cc84
There was a problem hiding this comment.
No worries, some repos have stricter tests than others 😅
|
If this is accepted I would like to see 2 new additions.
|
What for?
Do you mean it should be renamed? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thanks for the PR @stefanbuck :) |
| 'https://github.com/settings/replies', | ||
| 'https://github.com/settings/replies/88491/edit', |
There was a problem hiding this comment.
| 'https://github.com/settings/replies', | |
| 'https://github.com/settings/replies/88491/edit', | |
| ...collect.get('isRepliesSettings') as string[], |
There was a problem hiding this comment.
I'll include that in my other PR
Add
isUserSettingsdetector.Test url:
https://github.com/settings
btw. the live demo is super helpful 👏