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.

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

Improve and fix remove-projects-tab #3103

Merged

Conversation

yakov116
Copy link
Member

@yakov116 yakov116 commented May 18, 2020

Split remove-projects-tab into 2 parts.

  1. addNewProjectLink()
  2. removeProjectsTab()

Test on:
https://github.com/microsoft
https://github.com/twbs/bootstrap
https://github.com/babel

Exclude settings from org `set-default-repo...`
@fregante
Copy link
Member

fregante commented May 18, 2020

should isOrgRepoWithAccess be moved to the other module

Yes, but this is not an organization repo check, it's an organization check. It applies to: https://github.com/babel

I suggest renaming them to canUserEditRepo (from isRepoWithAccess) and canUserEditOrganization

isRepoWithAccess should still stay, but as a deprecated alias:

/** @deprecated use canUserEditRepo */
export const isRepoWithAccess = canUserEditRepo

@fregante
Copy link
Member

This whole feature can be rewritten in 2 init functions:

  1. addNewProjectLink
  2. removeProjectsTab

They have very different conditions, however both depend on !projectsTab, I think:

function getProjectsTab() {
	return elementReady([
		'[data-hotkey="g b"]', // In organizations and repos
		'.user-profile-nav [href$="?tab=projects"]' // In user profiles
	].join());
}

@yakov116 yakov116 marked this pull request as draft May 18, 2020 14:58
@yakov116
Copy link
Member Author

Blocked by refined-github/github-url-detection#3

@yakov116 yakov116 changed the title Use page detect for remove-projects-tab Rewrite remove-projects-tab May 18, 2020
@yakov116
Copy link
Member Author

This needs more work. Dont review this PR yet (But I can still use refined-github/github-url-detection#3)

@yakov116 yakov116 marked this pull request as ready for review May 18, 2020 20:33
@yakov116 yakov116 requested a review from fregante May 18, 2020 20:33
@fregante fregante marked this pull request as draft May 18, 2020 22:37
source/features/remove-projects-tab.tsx Outdated Show resolved Hide resolved
@yakov116
Copy link
Member Author

Updated logic and first post.

Dont have a org with projects to test on.

@yakov116 yakov116 marked this pull request as ready for review May 19, 2020 14:16
@yakov116

This comment has been minimized.

source/features/remove-projects-tab.tsx Outdated Show resolved Hide resolved
@fregante fregante assigned fregante and unassigned fregante May 19, 2020
@fregante
Copy link
Member

This works, but there's a little big problem: counters are loaded after dom-ready so sometimes the tab is removed even on https://github.com/microsoft

@fregante fregante self-assigned this May 19, 2020
@fregante fregante removed their assignment May 19, 2020
@fregante
Copy link
Member

This seems to work. Thoughts?

@yakov116
Copy link
Member Author

yakov116 commented May 19, 2020

Awesome! Very clean (love the emojis) 😄

@fregante fregante added the bug label May 19, 2020
@fregante fregante changed the title Rewrite remove-projects-tab Improve and fix remove-projects-tab May 19, 2020
@fregante fregante merged commit 37658a7 into refined-github:master May 19, 2020
@fregante
Copy link
Member

🙌

@yakov116 yakov116 deleted the use_page_detect_remove_projects_tab branch May 19, 2020 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants