Skip to content
Merged
3 changes: 3 additions & 0 deletions public/assets/icon-confirm.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions public/assets/icon-external.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions public/assets/nudge/icon-circle-dismiss.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion public/assets/tools/gigs.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion public/assets/tools/sprite.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/lib/app-context/profile-completion.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export interface ProfileCompletionData {
handle: string
percentComplete: number
showToast: string
dateFields?: [string, Date][]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The type dateFields?: [string, Date][] suggests an array of tuples, each containing a string and a Date. Consider using a more descriptive type alias or interface for the tuple to improve readability and maintainability, especially if this structure is used in multiple places.

}
2 changes: 1 addition & 1 deletion src/lib/components/modals/Modal.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
border: 1px solid;
border-radius: 1px;
color: #e9e9e9;
margin: 24px 0;
margin: 12px 0;
width: 100%;
display: block;
}
Expand Down
8 changes: 7 additions & 1 deletion src/lib/components/sticky/Sticky.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
let className = '';
export { className as class };
export let yOffset = 0;
export let delayYOffset = 0;

let elRef: HTMLElement | undefined;
let placeholderRef: HTMLElement | undefined;
let elYOffset = 0;

function handleScroll() {
Expand All @@ -15,7 +17,10 @@
}
const { scrollY } = window;

const isFixed = (scrollY + yOffset - elYOffset) >= 0;
const isFixed = (scrollY + yOffset - elYOffset - delayYOffset) >= 0;
if (placeholderRef) {
Object.assign(placeholderRef.style, {height: isFixed ? `${elRef.offsetHeight}px` : 0});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Using Object.assign to modify the style object directly can lead to unexpected behavior if other styles are applied dynamically elsewhere. Consider using style.height = ... for clarity and to avoid potential side effects.

}
elRef.classList.toggle(styles.sticky, isFixed);
}

Expand All @@ -31,6 +36,7 @@
})
</script>

<div bind:this={placeholderRef}></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ design]
The placeholder <div> is being used to maintain layout when the sticky element is fixed. Ensure that this placeholder does not interfere with other layout elements or cause unexpected shifts in the layout.

<div bind:this={elRef} class={className} style="--top-offset: {yOffset}px">
<slot />
</div>
2 changes: 1 addition & 1 deletion src/lib/components/tool-selector/ToolMenu.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@

.toolSection.talent & {
flex-direction: column;
max-height: 380px;
max-height: 420px;
}

@include mobile {
Expand Down
37 changes: 13 additions & 24 deletions src/lib/components/user-area/UserArea.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,20 @@

debounce = user.handle;

if (!DISABLE_NUDGES) {
const completednessData = await fetchUserProfileCompletedness(user, true);
if (!completednessData) {
return;
}
$ctx.auth = {
...$ctx.auth,
profileCompletionData: {
completed: completednessData.data?.percentComplete === 100,
handle: completednessData.handle,
percentComplete: completednessData.data?.percentComplete,
showToast: completednessData.showToast,
},
};
} else {
$ctx.auth = {
...$ctx.auth,
profileCompletionData: {
completed: true,
handle: user?.handle,
percentComplete: 0,
showToast: "",
},
};
const completednessData = await fetchUserProfileCompletedness(user, true);
if (!completednessData) {
return;
}
$ctx.auth = {
...$ctx.auth,
profileCompletionData: {
completed: completednessData.data?.percentComplete === 100,
handle: completednessData.handle,
percentComplete: completednessData.data?.percentComplete,
showToast: DISABLE_NUDGES ? '' : completednessData.showToast,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[💡 maintainability]
The use of a ternary operator to set showToast based on DISABLE_NUDGES is correct, but it might be more maintainable to handle this logic outside of the object literal for clarity, especially if more conditions are added in the future.

dateFields: completednessData.data?.dateFields,
},
};

setTimeout(() => debounce = '', 100);
}
Expand Down
1 change: 1 addition & 0 deletions src/lib/config/hosts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ export const TALENT_SEARCH_HOST: string = `https://talent-search.${TC_DOMAIN}`;
export const ACCOUNT_SETTINGS_HOST: string = `https://account-settings.${TC_DOMAIN}`;
export const WALLETAPP_HOST: string = `https://wallet.${TC_DOMAIN}`;
export const COPILOT_PORTAL_HOST: string = `https://copilots.${TC_DOMAIN}`;
export const ENGAGEMENT_PORTAL_HOST: string = `https://engagements.${TC_DOMAIN}`;
7 changes: 7 additions & 0 deletions src/lib/config/nav-menu/all-nav-items.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
WALLETAPP_HOST,
WORK_MANAGER_HOST,
AUTH0_AUTHENTICATOR_URL,
ENGAGEMENT_PORTAL_HOST,
} from '..';

export const allNavItems: {[key: string]: NavMenuItem} = {
Expand Down Expand Up @@ -111,6 +112,12 @@ export const allNavItems: {[key: string]: NavMenuItem} = {
icon: 'challenges',
description: 'Compete and earn money',
},
engagementsApp: {
label: 'Engagement Portal',
url: ENGAGEMENT_PORTAL_HOST,
icon: 'gigs',
description: 'Work directly with clients',
},
discordApp: {
label: 'Discord',
url: 'https://discord.com/invite/topcoder',
Expand Down
1 change: 1 addition & 0 deletions src/lib/config/nav-menu/tool-selector-nav-items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export const toolSelectorNavItems: NavMenuItem = {
groupOrder: 2,
children: [
allNavItems.challengesApp,
allNavItems.engagementsApp,
allNavItems.review,
allNavItems.payments,
allNavItems.copilotPortal,
Expand Down
33 changes: 25 additions & 8 deletions src/lib/functions/profile-nudges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,18 @@ import { DISABLE_NUDGES, NUDGES_DISABLED_HOSTS } from "lib/config/profile-toasts

import { getRequestAuthHeaders } from "./auth-jwt";

export function isOnHost(host: string): boolean {
const locationHostname = window?.location.hostname ?? ''
return !!host.match(new RegExp(`^https?:\/\/${locationHostname}`, 'i'));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[❗❗ correctness]
The regular expression in isOnHost does not correctly match the host. The ^https?:\/\/ pattern is unnecessary and incorrect for matching hostnames. Consider using ^${locationHostname}$ to match the hostname exactly.

}

/**
* Check if we're on a domain that should not show the profile nudges
* @returns Boolean
*/
export function dismissNudgesBasedOnHost(): boolean {
export function dismissNudgesBasedOnHost(exceptHost?: string): boolean {
// Ue the new flag to disable the profile nudges completely (PS-267)
const locationHostname = window?.location.hostname ?? ''
return DISABLE_NUDGES ||
(!!NUDGES_DISABLED_HOSTS.find(host => (
host.match(new RegExp(`^https?:\/\/${locationHostname}`, 'i'))
)));
return DISABLE_NUDGES || NUDGES_DISABLED_HOSTS.filter(h => !exceptHost || h !== exceptHost).some(isOnHost);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ performance]
Using filter followed by some in dismissNudgesBasedOnHost can be inefficient. Consider using some directly with a combined condition to improve performance.

}

// store fetched data in a local cache (de-duplicate immediate api calls)
Expand All @@ -25,7 +26,7 @@ export interface ProfileCompletednessResponse {
showToast: string
data: {
percentComplete: number
}
} & {[key: string]: any}
}

/**
Expand Down Expand Up @@ -55,13 +56,29 @@ export const fetchUserProfileCompletedness = async (user: AuthUser, force = fals
const request = fetch(requestUrl, {headers: {...getRequestAuthHeaders()}});

const response = await (await request).json();

const responseData = response.data ?? {};
const dateFields = Object.keys(responseData)
.filter(k => k.endsWith('LastUpdateDate') || k === 'lastProfileConfirmationDate')
.map((key) => [key, new Date(responseData[key])]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The conversion of date strings to Date objects in fetchUserProfileCompletedness may result in invalid dates if the input is not a valid date string. Consider adding validation to ensure the date strings are valid before conversion.


resolve({
...response,
data: {
...response.data,
...responseData,
dateFields,
percentComplete: (response?.data?.percentComplete ?? 0) * 100,
},
});

return localCache[cacheKey];
}

export const confirmProfileData = async (userHandle: string) => {
const requestUrl: string = `${TC_API_HOST}/members/${userHandle}/confirmProfile`;
const request = fetch(requestUrl, {method: 'POST', headers: {...getRequestAuthHeaders()}});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[❗❗ correctness]
The fetch call in confirmProfileData does not handle potential errors or non-200 HTTP responses. Consider adding error handling to manage network failures or unexpected response statuses.


const response = await (await request).json();

return response.data;
}
30 changes: 30 additions & 0 deletions src/lib/nudge-app/Nudge.module.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
@use 'lib/styles/mixins.scss' as *;

.bannerOuter,
.nudgeOuter {
position: absolute;
top: calc(var(--uninav-header-height) + var(--top-offset));
Expand All @@ -13,6 +14,12 @@
}
}

.bannerOuter {
position: relative;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
The .bannerOuter class is defined twice with different properties. This could lead to unexpected behavior. Consider consolidating these definitions to ensure clarity and maintainability.

top: 0;
height: min-content;
}

.nudgeWrap {
position: absolute;
display: flex;
Expand Down Expand Up @@ -41,3 +48,26 @@
width: 100%;
}
}

.bannerWrap {
display: flex;
width: 100%;
background: #60267d;
color: #fff;
}

.bannerInner {
width: 100%;
margin: 0 auto;
@include maxViewWidth;
padding: 6px 32px;

@include tablet {
padding: 6px 16px;
}

@include mobile {
padding: 0 2px;
width: 100%;
}
}
27 changes: 25 additions & 2 deletions src/lib/nudge-app/Nudge.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
import { getAppContext } from 'lib/app-context';
import Toastr from './components/Toastr.svelte';
import styles from './Nudge.module.scss';
import type { ToastType } from 'lib/config/profile-toasts.config';
import { toastsMeta, type ToastType } from 'lib/config/profile-toasts.config';
import { getToast, hideToast } from './toast-manager';
import Sticky from 'lib/components/sticky/Sticky.svelte';
import type { ProfileCompletionData } from 'lib/app-context/profile-completion.model';
import { getBanner, hide as hideBanner } from './banner-manager';
import Banner from './components/Banner.svelte';

const ctx = getAppContext();

Expand All @@ -16,20 +18,41 @@
} = $ctx.auth)

let toast: ToastType | undefined;
let banner: {key: string, date: Date} | undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Consider defining a type for banner instead of using an inline type {key: string, date: Date}. This will improve maintainability and readability, especially if the banner structure is used elsewhere.


function dismissToast() {
toast = undefined;
hideToast();
}

function dismissBanner() {
banner = undefined;
hideBanner();
}

$: toast = isReady ? getToast(profileCompletionData as ProfileCompletionData) : undefined;
$: banner = isReady ? getBanner(profileCompletionData as ProfileCompletionData) : undefined;
</script>

{#if banner}
<Sticky class={styles.bannerOuter} delayYOffset={92}>
<div class={styles.bannerWrap}>
<div class={styles.bannerInner}>
<Banner
userHandle={user?.handle ?? ''}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that user?.handle ?? '' is the desired fallback behavior. If user.handle is critical for the Banner component, consider handling the absence of user.handle more explicitly.

banner={banner}
on:dismiss={dismissBanner}
/>
</div>
</div>
</Sticky>
{/if}

{#if toast}
<Sticky class={styles.nudgeOuter} yOffset={10}>
<div class={styles.nudgeWrap}>
<div class={styles.nudgeInner}>
<Toastr userhandle={user.handle} toast={toast} on:dismiss={dismissToast} />
<Toastr userhandle={user?.handle ?? ''} toast={toast} on:dismiss={dismissToast} />

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
Ensure that user?.handle ?? '' is the desired fallback behavior. If user.handle is critical for the Toastr component, consider handling the absence of user.handle more explicitly.

</div>
</div>
</Sticky>
Expand Down
72 changes: 72 additions & 0 deletions src/lib/nudge-app/banner-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import type { ProfileCompletionData } from 'lib/app-context/profile-completion.model';
import { checkCookie, getCookieValue, setCookie } from '../utils/cookies';
import { dismissNudgesBasedOnHost } from '../functions/profile-nudges';
import { PROFILE_HOST } from 'lib/config';

const COOKIE_NAME = 'uni-profilereminder-banner-shown';
const COOKIE_ACTIVE_PERIOD_DAYS = 3;
const PROFILE_UPDATE_REMINDER_PERIOD_DAYS = 3*30;

const DAY = 24 * 3600 * 1000;
const _30DAYS = 30 * DAY;

function isDismissed() {
return checkCookie(COOKIE_NAME, 'hidden');
}

function getLastSeen() {
return getCookieValue(COOKIE_NAME);
}

const isOlderThanTreshold = (date: Date | number, treshold: number): boolean => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[💡 readability]
There is a typo in the function name isOlderThanTreshold. It should be isOlderThanThreshold. This could lead to confusion and should be corrected for clarity.

const diffInDays = Math.floor((Date.now() - +date) / DAY);
return diffInDays >= treshold;
}

/**
* @param completednessData
* @returns
*/
export const getBanner = (completednessData: ProfileCompletionData) => {
if (dismissNudgesBasedOnHost(PROFILE_HOST) || !completednessData || isDismissed()) {
return;
}

const fields = completednessData.dateFields ?? [];
const updatableProfileFields: [string, Date][] = fields
.filter(([k]) => k.endsWith('LastUpdateDate'))
.map(([k, d]) => [k.replace(/LastUpdateDate$/, ''), d]);
const lastProfileConfirmationDate = fields.find(([k]) => k === 'lastProfileConfirmationDate')?.[1];

const sorted = updatableProfileFields
.sort((a, b) => +a[1] - +b[1]);


const lastUpdate = +sorted[sorted.length - 1][1];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[❗❗ correctness]
Accessing sorted[sorted.length - 1][1] without checking if sorted is empty could lead to an error. Consider adding a check to ensure sorted has elements before accessing.

if (!lastUpdate) {
return;
}

const lastUpdateOrCofirmDate = lastProfileConfirmationDate ? Math.max(lastUpdate, +lastProfileConfirmationDate) : lastUpdate;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[💡 readability]
There is a typo in the variable name lastUpdateOrCofirmDate. It should be lastUpdateOrConfirmDate. This could lead to confusion and should be corrected for clarity.

if (!lastUpdateOrCofirmDate || !isOlderThanTreshold(lastUpdateOrCofirmDate, PROFILE_UPDATE_REMINDER_PERIOD_DAYS)) {
setCookie(COOKIE_NAME, '', 0);
return;
}

let lastSeen = getLastSeen();
if (!lastSeen) {
const oldFields = updatableProfileFields.filter(([,d]) => isOlderThanTreshold(d, PROFILE_UPDATE_REMINDER_PERIOD_DAYS)).map(([k]) => k);
const fieldKey = oldFields.length ? oldFields[0] : undefined;
const field = updatableProfileFields.find(f => f[0] === fieldKey) ?? [];
lastSeen = JSON.stringify({key: field[0], date: field[1]});
if(lastSeen) {
setCookie(COOKIE_NAME, lastSeen, COOKIE_ACTIVE_PERIOD_DAYS);
}
}

return JSON.parse(lastSeen);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[⚠️ correctness]
The return statement return JSON.parse(lastSeen); assumes lastSeen is always a valid JSON string. Consider adding error handling to manage potential JSON parsing errors.

}

export const hide = () => {
setCookie(COOKIE_NAME, 'hidden', COOKIE_ACTIVE_PERIOD_DAYS);
}
Loading
Loading