TUP-703 TACC Home Banner Links Open in New Window#805
Conversation
| //shouldForceSetTarget could be used in this if statement if passing in options. | ||
| if (!isInternalLink || isMailto ) { |
There was a problem hiding this comment.
Yup. (Thanks for the comment/reminder.) We avoid shouldForceSetTarget for now, cuz it causes the bug.
I'll check that no client needs that feature, and if one does, how to not require this script to handle that.
There was a problem hiding this comment.
Update: No client needs shouldForceSetTarget. Celebrate with Fire: You may burn all code and variables relevant to it e.g. pathsToExernalSite, pathsToForceSetTarget. Bonus: You may clean up even more cruft code, i.e. perform CMD-87, at your leisure, now or later.
wesleyboar
left a comment
wesleyboar
left a comment
There was a problem hiding this comment.
Looks good. Please delete shouldForceSetTarget and related code.
| //shouldForceSetTarget could be used in this if statement if passing in options. | ||
| if (!isInternalLink || isMailto ) { |
There was a problem hiding this comment.
Update: No client needs shouldForceSetTarget. Celebrate with Fire: You may burn all code and variables relevant to it e.g. pathsToExernalSite, pathsToForceSetTarget. Bonus: You may clean up even more cruft code, i.e. perform CMD-87, at your leisure, now or later.
| /** | ||
| * Whether to log debug info to console | ||
| * @const {string} | ||
| */ | ||
| const SHOULD_DEBUG = window.DEBUG; |
There was a problem hiding this comment.
Woah. Control the burn. The SHOULD_DEBUG is still used. Also, the jsDoc block (a docblock) is still accurate.
| /** | ||
| * Set external links (automatically discovered) to open in new tab | ||
| * @param {object} [options] - Optional parameters | ||
| * @param {array.<string>} [options.pathsToExernalSite=[]] - (DEPRECATED) A list of relative URL paths that should be treated like external URLs | ||
| * @param {array.<string|RegExp>} [options.pathsToForceSetTarget=[]] - A list of relative URL paths (or patterns) that should trigger setting a target | ||
| * @param {HTMLElement|Document} [options.scopeElement=document] - The element within which to search for links | ||
| * @param {setTargetCallback} [options.setTargetCallback] - A callback for after a target is set | ||
| */ |
There was a problem hiding this comment.
Please retain the docblock sans @param lines. The function description is still accurate.
Docblocks are written in jsDoc format. They provide manual definitions (in lieu of TypeScript), and can be used to auto-generate documentation e.g. vite.js on jsdocs.io.
I msitakenly requested this be restored. My bad. (First time using Github on phone, so I misread.) https://github.com/TACC/Core-CMS/pull/805/files#r1509226199
2 participants
Overview
Prevent banner links from opening in new windows.
Related
Changes
Changes what is considered an external link. Looks for the host name with or without www., then opens in new window if it isn't matching to the document's host.
Testing
A) If you'd like to troubleshoot in TUP-UI Repo:
B) If you'd like to just test these changes in TUP-UI using Core_CMS
UI
…
Notes
The
shouldForceSetTargetfunction below:does not allow external links to open in a new window.
For instance. If you're in localhost, and have a
tacc.utexas.edulink - it should open in a new window because it doesn't match the host.This might be something you test in tup-ui repo using the A) testing steps above