Fix for percent-encoded fragments not changing tab on load and the hashchange event not working #210
Conversation
WalkthroughThe changes focus on improving URL hash handling in the TabberNeue module. The primary modification involves decoding percent-encoded characters in URL hashes before processing them. This ensures more accurate tab selection when navigating using URL fragments by first using Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser
participant TabberModule
User->>Browser: Navigate with encoded URL hash
Browser->>TabberModule: Trigger hashchange event
TabberModule->>TabberModule: Decode percent-encoded hash
TabberModule->>TabberModule: Escape decoded hash
TabberModule->>TabberModule: Select appropriate tab
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/ext.tabberNeue/ext.tabberNeue.js (1)
489-490: Add missing semicolon.Add a semicolon after the
percentDecodeFragment(hash)call to fix the eslint error.- const decodedHash = mw.util.percentDecodeFragment( hash ) + const decodedHash = mw.util.percentDecodeFragment( hash );🧰 Tools
🪛 eslint
[error] 489-490: Missing semicolon.
(semi)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/ext.tabberNeue/ext.tabberNeue.js(2 hunks)
🧰 Additional context used
📓 Learnings (2)
📓 Common learnings
Learnt from: alistair3149
PR: StarCitizenTools/mediawiki-extensions-TabberNeue#189
File: modules/ext.tabberNeue/ext.tabberNeue.js:427-430
Timestamp: 2024-11-16T08:18:43.156Z
Learning: In `modules/ext.tabberNeue/ext.tabberNeue.js`, within the `getActiveTab` method of the `TabberBuilder` class, the check `if (idFromUrlHash === escapedHash)` ensures that if replacing the `'tabber-tabpanel-'` prefix does not modify the `escapedHash`, then the hash does not represent a valid tabpanel ID, and the current check is sufficient.
modules/ext.tabberNeue/ext.tabberNeue.js (1)
Learnt from: alistair3149
PR: StarCitizenTools/mediawiki-extensions-TabberNeue#189
File: modules/ext.tabberNeue/ext.tabberNeue.js:427-430
Timestamp: 2024-11-16T08:18:43.156Z
Learning: In `modules/ext.tabberNeue/ext.tabberNeue.js`, within the `getActiveTab` method of the `TabberBuilder` class, the check `if (idFromUrlHash === escapedHash)` ensures that if replacing the `'tabber-tabpanel-'` prefix does not modify the `escapedHash`, then the hash does not represent a valid tabpanel ID, and the current check is sufficient.
🪛 eslint
modules/ext.tabberNeue/ext.tabberNeue.js
[error] 489-490: Missing semicolon.
(semi)
🔇 Additional comments (2)
modules/ext.tabberNeue/ext.tabberNeue.js (2)
427-428: LGTM! Good fix for handling percent-encoded fragments.The addition of
mw.util.percentDecodeFragmentensures proper handling of percent-encoded characters in URL fragments before they are escaped, which fixes the tab selection issue on load.
489-492: LGTM! Good fix for hashchange event.The changes correctly handle percent-encoded fragments in the hashchange event handler, maintaining consistency with the
getActiveTabmethod's implementation.🧰 Tools
🪛 eslint
[error] 489-490: Missing semicolon.
(semi)
|
Caution An unexpected error occurred while opening a pull request: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
Fix for #209 by passing the fragment first through mw.util.percentDecodeFragment.
Also fixes the
hashchangeevent to even work, in it's current form it never works since it appendstabber-tab-onto the fragment instead of replacingtabber-tabpanel-.Summary by CodeRabbit