Skip to content

[PROD] - Security fixes & clean up #394

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.

Sign up for GitHub

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

Merged
kkartunov merged 3 commits into from
Dec 10, 2025
Merged

[PROD] - Security fixes & clean up #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1b72923
PM-2749 - upgrade major node modules
vas3a Dec 9, 2025
680e530
remove nudge
vas3a Dec 9, 2025
af5248c
Merge pull request #393 from topcoder-platform/PM-2749_major-update-m…
kkartunov Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .nvmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
v22.13.1
3,315 changes: 1,403 additions & 1,912 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

24 changes: 11 additions & 13 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@
"types": "./types/src/main.d.ts",
"repository": "https://github.com/topcoder-platform/universal-navigation.git",
"scripts": {
"build:nudge": "APP_BUILD_TARGET=nudge vite build --mode=production",
"build:base": "vite build --mode=production && npx rimraf ./types && npm run types",
"build": "npm run build:base && npm run build:nudge",
"build": "vite build --mode=production && npx rimraf ./types && npm run types",
"check": "svelte-check --tsconfig ./tsconfig.json",
"demo:dist": "npx http-server --cors --host local.topcoder-dev.com --port 8083 ./dist",
"demo:marketing": "npx http-server --host local.topcoder-dev.com --port 8081 ./demo/marketing -P http://local.topcoder-dev.com:8081? -o /",
Expand All @@ -21,15 +19,15 @@
"types": "./node_modules/typescript/bin/tsc -p ./tsconfig.json --declaration --emitDeclarationOnly --outDir types"
},
"devDependencies": {
"@sveltejs/vite-plugin-svelte": "^1.1.0",
"@tsconfig/svelte": "^3.0.0",
"sass": "^1.56.1",
"svelte": "^3.52.0",
"svelte-check": "^2.9.2",
"svelte-preprocess": "^4.10.7",
"tslib": "^2.4.0",
"typescript": "^4.6.4",
"vite": "^3.2.3",
"vite-plugin-css-injected-by-js": "^2.1.1"
"@sveltejs/vite-plugin-svelte": "^3.1.2",
"@tsconfig/svelte": "^5.0.6",
"sass": "^1.95.0",
"svelte": "^4.2.20",
"svelte-check": "^4.3.4",
"svelte-preprocess": "^6.0.3",
"tslib": "^2.8.1",
"typescript": "^5.9.3",
"vite": "^5.4.21",
"vite-plugin-css-injected-by-js": "^3.5.2"
}
}
4 changes: 2 additions & 2 deletions src/lib/app-context/auth-config.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import type { AuthUser } from "./auth-user.model"
import type { ProfileCompletionData } from "./profile-completion.model"

export interface AuthConfig {
user: AuthUser
profileCompletionData: ProfileCompletionData
user?: AuthUser
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
Making user optional in the AuthConfig interface could lead to runtime errors if the rest of the codebase assumes user is always defined. Ensure that all usages of AuthConfig handle the case where user is undefined.

profileCompletionData?: ProfileCompletionData
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[❗❗ correctness]
Making profileCompletionData optional in the AuthConfig interface could lead to runtime errors if the rest of the codebase assumes profileCompletionData is always defined. Ensure that all usages of AuthConfig handle the case where profileCompletionData is undefined.

autoFetchUser?: boolean
ready: boolean
signIn: () => void
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Accordion.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;

.accordionWrap {
display: flex;
Expand Down
12 changes: 9 additions & 3 deletions src/lib/components/Accordion.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
import { getPublicPath } from 'lib/utils/paths';
import styles from './Accordion.module.scss';
export let activeRoute: NavMenuItem = undefined;
export let items: NavMenuItem[];
export let activeRoute: NavMenuItem | undefined = undefined;
export let items: NavMenuItem[] = [];
export let style: 'primary'|'secondary'|undefined = undefined;
const toggledItems: {[key: string]: boolean} = {};
Expand Down Expand Up @@ -42,7 +42,13 @@
{item.label}
</a>
{#if item.children?.length}
<span class={styles.itemTrigger} on:click={() => toggleItem(item)} on:keydown={() => {}}>
<span
class={styles.itemTrigger}
role="button"
tabindex="0"
on:click={() => toggleItem(item)}
on:keydown={(ev) => ev.key === 'Enter' && toggleItem(item)}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ accessibility]
The on:keydown event handler should also handle the 'Space' key to improve accessibility, as it is a common key for activating buttons. Consider updating the condition to (ev) => (ev.key === 'Enter' || ev.key === ' ') && toggleItem(item).

>
<img src={iconUrl} alt="^" />
</span>
{/if}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Banner.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better modularity and to avoid global namespace pollution. However, using as * can still lead to namespace conflicts. Consider using a specific namespace instead of * to prevent potential issues in larger projects.


$btnSize: 32px;

Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Button.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better modularity and avoiding global namespace pollution. However, using as * imports all variables into the global scope, which can lead to similar issues as @import. Consider importing only the necessary variables or using a namespace to avoid potential conflicts.


.btn {
all: unset;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Button.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
export let variant: ButtonVariant = ''
export let disabled: boolean = false;
export let size: 'sm'|'md'|'md-0' = 'sm';
export let ref: HTMLElement = null;
export let ref: HTMLElement | null = null;

</script>

Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/LinksMenu.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * negates some of these benefits by making all variables and mixins globally available. Consider using a namespace instead to maintain encapsulation and improve maintainability.


.linksMenuWrap {
display: flex;
Expand Down
18 changes: 9 additions & 9 deletions src/lib/components/LinksMenu.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,29 +10,29 @@
export let ref: Element | undefined = undefined;
export let style: "primary" | "secondary" | "tertiary" | 'cta';
export let menuItems: NavMenuItem[];
export let activeRoute: NavMenuItem = undefined;
export let menuItems: NavMenuItem[] = [];
export let activeRoute: NavMenuItem | undefined = undefined;
export let activeRoutePath: NavMenuItem[] = [];
export let vertical: boolean = false;
export let navigationHandler: NavigationHandler | undefined = undefined;
let hoveredMenuItem: NavMenuItem = undefined;
let hoveredElement: HTMLElement = undefined;
let hoveredMenuItem: NavMenuItem | undefined = undefined;
let hoveredElement: HTMLElement | undefined = undefined;
let isPopupMenuActive: boolean = false;
function isActiveMenu(menuItem: NavMenuItem, activeMenuItem: NavMenuItem) {
function isActiveMenu(menuItem: NavMenuItem, activeMenuItem?: NavMenuItem) {
return activeMenuItem?.url !== undefined && menuItem.url === activeMenuItem?.url
}
function itemHasHoverMenu(menuItem: NavMenuItem) {
return menuItem.children?.length || menuItem.description;
return !!(menuItem.children?.length) || !!menuItem.description;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 readability]
The use of double negation !! is unnecessary here and can reduce readability. Consider using Boolean() for clarity.

}
const handleMouseover = (menuItem: NavMenuItem) => async (ev) => {
const handleMouseover = (menuItem: NavMenuItem) => async (ev: Event) => {
if (!itemHasHoverMenu(menuItem)) {
return;
}
hoveredElement = ev.target;
hoveredElement = ev.currentTarget as HTMLElement ?? undefined;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 readability]
Using ev.currentTarget as HTMLElement ?? undefined is redundant. ev.currentTarget should always be an HTMLElement in this context. Consider simplifying to ev.currentTarget as HTMLElement.

hoveredMenuItem = menuItem;
};
Expand All @@ -53,7 +53,7 @@
if (typeof navigationHandler === 'function') {
ev.preventDefault()
navigationHandler({label: '', path: menuItem.url, isMarketingUrl: !!menuItem.marketingPathname});
navigationHandler({label: menuItem.label ?? '', path: menuItem.url, isMarketingUrl: !!menuItem.marketingPathname});
}
}
</script>
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/Maintenance.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for modularity and avoiding global namespace pollution. However, using as * reintroduces the risk of namespace conflicts. Consider using a specific namespace instead of * to maintain modularity benefits.


.bannerWrap {
position: relative;
Expand Down
6 changes: 3 additions & 3 deletions src/lib/components/MobileMenu.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
const closeMenuIcon = getPublicPath(`/assets/icon-close.svg`);
export let direction: 'x'|'y';
export let handleClose = () => {};
export let handleClose: () => void = () => {};
let animParams: FlyParams = {duration: 200};
$: animParams[direction] = direction === 'x' ? -320 : 50;
Expand All @@ -29,7 +29,7 @@
return () => window.removeEventListener('resize', updateVh);
})
function toggleOverflow(toggle) {
function toggleOverflow(toggle: boolean) {
Object.assign(document.body.style, {overflow: toggle ? 'hidden' : ''});
window.scrollTo(0, 0);
}
Expand All @@ -43,7 +43,7 @@

<div class={styles.mobileMenuWrap} transition:fade={{duration: 200}}>
<TopNavbar style="primary" showLogo={false}>
<div class={styles.closeIcon} slot="right" on:click={handleClose} on:keydown={() => {}}>
<div class={styles.closeIcon} role="button" tabindex="0" slot="right" on:click={handleClose} on:keydown={() => {}}>
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ accessibility]
The on:keydown handler is currently an empty function. This could lead to unexpected behavior if users attempt to interact with the button using the keyboard. Consider implementing keyboard accessibility by handling key events such as Enter or Space to trigger handleClose.

<img src={closeMenuIcon} alt="close" />
</div>
</TopNavbar>
Expand Down
4 changes: 2 additions & 2 deletions src/lib/components/PopupMenu.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,13 @@
* Remove the targetKey & mouse event listeners
* @param el
*/
function unBindEvents(el: HTMLElement = targetEl) {
function unBindEvents(el: HTMLElement | undefined = targetEl) {
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The change to allow el to be undefined in the unBindEvents function signature is appropriate given the default value of targetEl. However, ensure that all calls to unBindEvents handle the possibility of undefined correctly, especially if the function is used elsewhere in the codebase.

if (!el) {
return
}
el.dataset.targetKey = undefined
delete el.dataset.targetKey
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using delete el.dataset.targetKey instead of setting it to undefined is a better approach as it removes the attribute entirely, which is more semantically correct and avoids leaving an empty attribute. Ensure this change is consistent with any logic that checks for the presence of data-target-key.

el.removeEventListener('mouseenter', handleMouseover)
}
Expand Down
6 changes: 3 additions & 3 deletions src/lib/components/SubMenu.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@import 'lib/styles/mixins.scss';
@import 'lib/styles/fonts.scss';
@import 'lib/styles/colors.scss';
@use 'lib/styles/mixins.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * defeats this purpose by exposing all members to the global scope. Consider using specific namespaces instead of as * to maintain encapsulation.

@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Using @use 'lib/styles/fonts.scss' as *; exposes all members globally, which can lead to conflicts and maintenance challenges. It's recommended to use a specific namespace to keep the scope limited and avoid potential issues.

@use 'lib/styles/colors.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The @use 'lib/styles/colors.scss' as *; statement exposes all members globally, which can lead to conflicts and maintenance challenges. Consider using a specific namespace to keep the scope limited and avoid potential issues.


$transitionInDelay: 15ms;

Expand Down
6 changes: 3 additions & 3 deletions src/lib/components/SubMenu.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
export let menuItems: NavMenuItem[] = [];
export let isHovering: boolean = false;
export let activeRoute: NavMenuItem = undefined;
export let activeRoute: NavMenuItem | undefined = undefined;
export let navigationHandler: NavigationHandler | undefined = undefined;
let elWrap: HTMLElement | undefined;
Expand All @@ -25,9 +25,9 @@
})
function handleNavigation(ev: MouseEvent, menuItem: NavMenuItem) {
if (typeof navigationHandler === 'function') {
if (typeof navigationHandler === 'function' && menuItem.url) {
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The added check menuItem.url ensures that menuItem.url is defined before attempting to use it in navigationHandler. This is a good improvement for preventing potential runtime errors. However, consider adding a more explicit error handling or logging mechanism if menuItem.url is undefined, to aid in debugging and provide better feedback.

ev.preventDefault()
navigationHandler({label: '', path: menuItem.url, isMarketingUrl: !!menuItem.marketingPathname});
navigationHandler({label: menuItem.label ?? '', path: menuItem.url, isMarketingUrl: !!menuItem.marketingPathname});
}
}
</script>
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/TcLogo.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/mixins.scss';
@use 'lib/styles/mixins.scss' as *;

.logo {
display: flex;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/TopNavbar.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/mixins.scss';
@use 'lib/styles/mixins.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better modularity and to avoid potential conflicts. However, using as * can negate some of the benefits of @use by exposing all variables and mixins globally. Consider importing only the specific mixins or variables needed to maintain encapsulation and prevent unintended side effects.


.topNavbarWrap {
position: relative;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/VerticalSeparator.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/mixins.scss';
@use 'lib/styles/mixins.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better modularity and to avoid namespace conflicts. However, using as * can negate some of these benefits by polluting the global namespace. Consider importing only the necessary mixins explicitly to maintain modularity and avoid potential conflicts.


.verticalSeparator {
display: block;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/forms/InputWrap.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/fonts.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * negates some of these benefits by exposing all variables and mixins globally. Consider using a specific namespace instead of * to maintain encapsulation and avoid potential conflicts.


.inputWrap {
background: #fff;
Expand Down
4 changes: 2 additions & 2 deletions src/lib/components/modals/Modal.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
@import 'lib/styles/fonts.scss';
@import 'lib/styles/mixins.scss';
@use 'lib/styles/fonts.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * effectively negates this benefit by importing all members into the global scope. Consider importing only the specific members needed to maintain encapsulation.

@use 'lib/styles/mixins.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Similar to the previous line, using @use 'lib/styles/mixins.scss' as * imports all members into the global scope, which can lead to namespace conflicts and reduced maintainability. It's better to import only the necessary members.


.modalWrap {
position: fixed;
Expand Down
10 changes: 5 additions & 5 deletions src/lib/components/modals/Modal.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
import { classnames } from 'lib/utils/classnames';
import styles from './Modal.module.scss';
export let isVisible: string = '';
export let isVisible: boolean = false;
export let title: string = '';
export let size: 'sm' = undefined;
export let size: 'sm' | undefined = undefined;
function toggleOverflow(toggle) {
function toggleOverflow(toggle: boolean) {
Object.assign(document.body.style, {overflow: toggle ? 'hidden' : ''});
}
Expand All @@ -18,15 +18,15 @@
{#if isVisible}
<div class={classnames(styles.modalWrap, $$props.class, size && `size-${size}`)}>
<div class={styles.modalContainer}>
<div class={styles.modalOverlay} transition:fade={{duration: 200}} on:click={() => isVisible = false} on:keydown={() => {}} />
<div class={styles.modalOverlay} role="button" tabindex="0" transition:fade={{duration: 200}} on:click={() => isVisible = false} on:keydown={() => {}} />
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 maintainability]
The on:keydown handler is currently an empty function. If this is intentional, consider removing it to avoid confusion, or implement the desired functionality.

<div class={styles.modalWindow} transition:fly={{y: 45, duration: 300}}>
<div class={styles.modalHeader}>
<h3 class={styles.modalTitle}>
{title}
</h3>
<button
class={styles.closeBtn}
on:click={() => isVisible = ''}
on:click={() => isVisible = false}
on:keydown={() => {}}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 maintainability]
The on:keydown handler is an empty function. If no functionality is intended, consider removing it to improve code clarity.

>
<svg xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="2"
Expand Down
6 changes: 6 additions & 0 deletions src/lib/components/sticky/Sticky.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,19 @@
let elYOffset = 0;
function handleScroll() {
if (!elRef) {
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 maintainability]
The check for elRef being undefined is a good addition to prevent errors, but consider initializing elRef to null instead of undefined for better clarity and consistency with DOM element references.

return;
}
const { scrollY } = window;
const isFixed = (scrollY + yOffset - elYOffset) >= 0;
elRef.classList.toggle(styles.sticky, isFixed);
}
onMount(() => {
if (!elRef) {
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[💡 maintainability]
The check for elRef being undefined is a good addition to prevent errors, but consider initializing elRef to null instead of undefined for better clarity and consistency with DOM element references.

return;
}
elYOffset = elRef.offsetTop;
handleScroll();
Expand Down
6 changes: 3 additions & 3 deletions src/lib/components/tool-selector/ToolMenu.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@import 'lib/styles/mixins.scss';
@import 'lib/styles/fonts.scss';
@import 'lib/styles/colors.scss';
@use 'lib/styles/mixins.scss' as *;
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Switching from @import to @use is a good practice for better encapsulation and avoiding global namespace pollution. However, using as * negates some of the benefits of @use by exposing all variables and mixins globally. Consider using specific namespaces or importing only what is necessary to maintain encapsulation.

@use 'lib/styles/fonts.scss' as *;
@use 'lib/styles/colors.scss' as *;

.toolMenuWrap {
width: 472px;
Expand Down
10 changes: 6 additions & 4 deletions src/lib/components/tool-selector/ToolMenu.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import InlineSvg from '../InlineSvg.svelte';
import styles from './ToolMenu.module.scss';
let navMenuItems = getToolSelectorItems();
let navMenuItems: NavMenuItem[] = getToolSelectorItems() ?? [];
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of the nullish coalescing operator (??) to default getToolSelectorItems() to an empty array is a good practice for ensuring navMenuItems is always iterable. However, ensure that getToolSelectorItems() is expected to return null or undefined in some cases, as this could mask potential issues where the function fails to return the expected data.

const toolIcon = getPublicPath('/assets/icon-tool.svg');
function hasCtas(item: NavMenuItem) {
Expand All @@ -18,13 +18,14 @@
<div class={styles.toolMenuWrap}>
<InlineSvg src="/assets/tools/sprite.svg" />
{#each navMenuItems as section, sectionIndex}
<div class={classnames(styles.toolSection, styles[section.label?.toLowerCase()])}>
{#if section}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The added if check for section ensures that the block is only rendered when section is truthy. This is a good safeguard, but verify that navMenuItems is expected to contain falsy values, as this could indicate an issue with data integrity.

<div class={classnames(styles.toolSection, section.label ? styles[section.label.toLowerCase()] : undefined)}>
<div class={styles.toolSectionTitle}>
{section.label}
</div>

<div class={styles.toolGroups}>
{#each section.children as group}
{#each section.children ?? [] as group}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using the nullish coalescing operator (??) to default section.children to an empty array is a good practice for ensuring the each block is always iterable. However, confirm that section.children is expected to be null or undefined, as this could hide potential data issues.

<div
class={classnames(styles.toolGroup, hasCtas(group) && styles.hasCtas)}
style:--order={group.groupOrder ?? ''}
Expand All @@ -36,7 +37,7 @@
{/if}

<div class={styles.toolNavItems}>
{#each group.children as navItem}
{#each group.children ?? [] as navItem}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ correctness]
The use of the nullish coalescing operator (??) to default group.children to an empty array ensures that the each block is always iterable. Ensure that group.children is expected to be null or undefined, as this could mask potential data issues.

<a
href={navItem.url}
class={classnames(styles.toolNavItem, navItem.type === 'cta' && 'navButton')}
Expand Down Expand Up @@ -74,5 +75,6 @@
{#if sectionIndex < navMenuItems.length-1}
<hr class={styles.toolMenuSpacer} />
{/if}
{/if}
{/each}
</div>
4 changes: 3 additions & 1 deletion src/lib/components/tool-selector/ToolSelector.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@
const imgUrl = getPublicPath('/assets/tool-trigger.svg');
const toolsIcons = getPublicPath('/assets/tools/sprite.svg');
let elRef: HTMLElement;
let elRef: HTMLElement | undefined;
let popupIsVisible: boolean;
</script>

<div
class={styles.wrap}
role="button"
tabindex="0"
bind:this={elRef}
on:click={() => popupIsVisible = true}
on:keydown={() => {}}
Copy link

@github-actions github-actions bot Dec 10, 2025

Choose a reason for hiding this comment

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

[⚠️ accessibility]
The on:keydown handler is currently a no-op. If the intention is to make the element accessible via keyboard, consider implementing meaningful keyboard interactions, such as toggling popupIsVisible on Enter or Space key presses.

Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/user-area/UserArea.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
@import 'lib/styles/mixins.scss';
@use 'lib/styles/mixins.scss' as *;

.userAreaWrap {
display: flex;
Expand Down
Loading
Loading