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

Make releases-tab non dependent of more-dropdown #3654

Merged
merged 4 commits into from
Oct 20, 2020

Conversation

yakov116
Copy link
Member

  1. LINKED ISSUES:
    Resolves Release-tab throws an error if more-dropdown is turned off #3652

  2. TEST URLS:
    None

  3. SCREENSHOT:
    None

@fregante
Copy link
Member

The dropdown is native, we’re not creating it. Perhaps the only problem is that there’s no native separator in there.

@yakov116 yakov116 added the bug label Oct 15, 2020
@fregante fregante marked this pull request as draft October 17, 2020 02:23
'data-menu-item': 'rgh-releases-item'
})
);
const moreDropdown = select('.dropdown-divider', repoNavigationBar);
Copy link
Member

@fregante fregante Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we should just match the logic on line 85, which simply appends it. We only need to slightly modify because we have a divider.

Thankfully we have an helper function that does this exactly. I think the only change needed is:

Suggested change
const moreDropdown = select('.dropdown-divider', repoNavigationBar);
appendBefore(
select('.js-responsive-underlinenav .dropdown-menu ul'),
'.dropdown-divider', // Won't exist if `more-dropdown` is disabled
createDropdownItem('Releases', `/${repoUrl}/releases`, {
'data-menu-item': 'rgh-releases-item'
})
);

This comment was marked as off-topic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works perfectly!

yakov116 and others added 2 commits October 18, 2020 23:17
Co-Authored-By: Fregante <opensource@bfred.it>
@yakov116 yakov116 marked this pull request as ready for review October 19, 2020 03:21
@fregante
Copy link
Member

fregante commented Oct 19, 2020

PR title needs update then it’s ok if it works

@yakov116 yakov116 changed the title Only add the release-tab to the more-dropdown if its enabled Make release-tab non dependent of more-dropdown Oct 19, 2020
@yakov116
Copy link
Member Author

How is it now?

@yakov116 yakov116 changed the title Make release-tab non dependent of more-dropdown Make releases-tab non dependent of more-dropdown Oct 20, 2020
@yakov116 yakov116 merged commit 8e9853f into refined-github:master Oct 20, 2020
@yakov116 yakov116 deleted the release_tab_check_for_more branch October 20, 2020 02:47
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.

Release-tab throws an error if more-dropdown is turned off
2 participants