Meta: Refactor custom tab highlighting#4918
Conversation
There was a problem hiding this comment.
This PR unifies some duplicated code between releases-tab and bugs-tab. Also the elementReady({stopOnDomReady:false}) is dropped in favor of a simple event listener. It seems to work pretty well.
@fregante you mentioned #4008 (comment), can you clarify? Does this mean adding an entry for the bugs tab in the more-dropdown-links menu?
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.
This comment has been minimized.
This comment has been minimized.
|
This could be simplified, for example instead of using events, you can just call a function to re-update the nav bar. Alternative we could use an additional function init () {
if (shouldBugsTabHaveFocus) {
// Remove focus from other tab
}
// Ensure that the bugs tab is consistent whether it needs it or not
bugsTab.classList.toggle('current', shouldBugsTabHaveFocus);
bugsTab.attributes.toggle('aria-current', shouldBugsTabHaveFocus); // Or whatever
}
const shouldBugsTabHaveFocus = mem((url: string) => {
///logic
}) |
That's already the case, the problem is ensuring that the A simpler solution could be to have a single un-deduplicated init that would:
|
append(bugsTab);
secondInit() // Call it manually, even if it's duplicate right now
That'd also work. As long as the pattern is preserved for |
cheap-glitch
left a comment
There was a problem hiding this comment.
Went with the single-init solution.
|
Up to you Just merge after 21.11.1 |
|
@refined-github/maintainers can someone else test this too, ideally in a browser other than Firefox? Thanks! |
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.
…/refined-github into `refactor-tab-highlighting`
Follow-up of #4914 (comment) and #4914 (comment)
Test URLs
Navigating between tabs in any repo
Screenshot
capture-1634213450.mp4