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

Fix more-dropdown on GHE #3962

Merged
merged 3 commits into from Feb 9, 2021
Merged

Fix more-dropdown on GHE #3962

merged 3 commits into from Feb 9, 2021

Conversation

cheap-glitch
Copy link
Member

@cheap-glitch cheap-glitch commented Feb 9, 2021

Thanks for contributing! 🍄

  1. LINKED ISSUES: Fixes more-dropdown: Cannot read property 'parentElement' of undefined #3953

  2. TEST URLS: Any repo page. The "Security" and "Insights" tabs should be moved to the dropdown.

My guess is that the bug is caused by the absence of the "Security" tab when doing:

onlyShowInDropdown('security-tab');
onlyShowInDropdown('insights-tab');

I can't test this on GHE, but checking for the existence of the tab element should fix this.

@fregante
Copy link
Member

fregante commented Feb 9, 2021

Thanks, this however silences errors for everyone. If the security tab doesn't existing on GHE, I'd rather see a if !isGHE, onlyShowInDropdown('security-tab')

@fregante fregante requested a review from busches February 9, 2021 18:19
@busches
Copy link
Member

busches commented Feb 9, 2021

I do not have access to this version of GHE to confirm. Hopefully @orrc or maybe @mrbusche can test.

@cheap-glitch
Copy link
Member Author

Thanks, this however silences errors for everyone. If the security tab doesn't existing on GHE, I'd rather see a if !isGHE, onlyShowInDropdown('security-tab')

Is the "Security" tab never present on GHE? If it's only missing sometimes, how about:

if (!tabItem && isGHE) {
	return;
}

@orrc
Copy link

orrc commented Feb 9, 2021

The last couple of versions of the Enterprise docs do mention the "Security tab" as being a thing — it's just not something that appears in any of the many repos I've checked on the one GHE install I have access to.

In any case, I tested this PR and the more-dropdown error is no longer present. 👍

@yakov116 yakov116 added the bug label Feb 9, 2021
@mrbusche
Copy link

mrbusche commented Feb 9, 2021

On GHE 2.22.6 I'm not seeing the Security tab with or without admin rights.
image

Co-authored-by: yakov116 <16872793+yakov116@users.noreply.github.com>
@yakov116 yakov116 merged commit 9862dee into refined-github:main Feb 9, 2021
@cheap-glitch cheap-glitch deleted the fix-more-dropdown-ghe branch February 10, 2021 00:06
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.

more-dropdown: Cannot read property 'parentElement' of undefined
6 participants