-
Notifications
You must be signed in to change notification settings - Fork 103
fix(condo): DOMA-13033 fix tooltip for miniapps #7330
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.
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,18 +19,20 @@ | |
| feature?: AvailableFeature | AvailableFeature[] | ||
| path?: string | ||
| skipTooltip?: boolean | ||
| b2bAppId?: string | ||
| } | ||
|
|
||
| export const NoSubscriptionTooltip: React.FC<NoSubscriptionTooltipProps> = ({ children, placement = 'right', feature: featureProp, path, skipTooltip }) => { | ||
| export const NoSubscriptionTooltip: React.FC<NoSubscriptionTooltipProps> = ({ children, placement = 'right', feature: featureProp, path, skipTooltip, b2bAppId }) => { | ||
| const intl = useIntl() | ||
| const router = useRouter() | ||
| const { organization } = useOrganization() | ||
| const { trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading } = useActivateSubscriptions() | ||
| const { isFeatureAvailable, hasSubscription } = useOrganizationSubscription() | ||
| const { isFeatureAvailable, hasSubscription, isB2BAppEnabled } = useOrganizationSubscription() | ||
| const [isActivating, setIsActivating] = useState(false) | ||
|
|
||
| const requiredFeature = path ? getRequiredFeature(path) : null | ||
| const feature = (featureProp || requiredFeature) as AvailableFeature | undefined | null | ||
| const isAppAvailableForTariff = b2bAppId ? isB2BAppEnabled(b2bAppId) : true | ||
|
|
||
| const FeatureLockedMessage = intl.formatMessage({ | ||
| id: 'subscription.warns.noActiveSubscription', | ||
|
|
@@ -50,14 +52,23 @@ | |
| }) | ||
|
|
||
| const hasActivatedAnyTrial = trialSubscriptions.length > 0 | ||
| const isAvailable = feature | ||
| ? Array.isArray(feature) | ||
| ? feature.every(f => isFeatureAvailable(f)) | ||
| : isFeatureAvailable(feature) | ||
| : false | ||
|
|
||
| const isAvailable = useMemo(() => { | ||
| if (b2bAppId) { | ||
| return isAppAvailableForTariff | ||
| } else if (feature) { | ||
| if (Array.isArray(feature)) { | ||
| return feature.every(isFeatureAvailable) | ||
|
Check failure on line 61 in apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx
|
||
| } else { | ||
| return isFeatureAvailable(feature) | ||
| } | ||
| } else { | ||
| return false | ||
| } | ||
| }, [b2bAppId, isAppAvailableForTariff, feature, isFeatureAvailable]) | ||
|
Comment on lines
+56
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent behavior when neither In However, in This inconsistency could lead to confusing behavior where the same scenario produces different outcomes depending on which component is used directly. Also applies to: 51-59 🤖 Prompt for AI Agents |
||
|
|
||
| const bestPlanWithFeature = useMemo(() => { | ||
| if (!feature || isAvailable) return null | ||
| if ((!feature && !b2bAppId) || isAvailable) return null | ||
|
|
||
| const plans = plansData?.result?.plans || [] | ||
|
|
||
|
|
@@ -66,9 +77,17 @@ | |
| const plan = planInfo?.plan | ||
| if (!plan) return false | ||
|
|
||
| const hasFeature = Array.isArray(feature) | ||
| ? feature.every(f => plan[f] === true) | ||
| : plan[feature] === true | ||
| let hasRequirement = false | ||
|
|
||
| if (b2bAppId) { | ||
| const enabledApps = plan.enabledB2BApps || [] | ||
| hasRequirement = enabledApps.includes(b2bAppId) | ||
| } else if (feature) { | ||
| hasRequirement = Array.isArray(feature) | ||
| ? feature.every(f => plan[f] === true) | ||
| : plan[feature] === true | ||
| } | ||
|
|
||
| const hasTrialDays = plan.trialDays > 0 | ||
| const prices = planInfo?.prices || [] | ||
| const hasPrice = prices.length > 0 | ||
|
|
@@ -80,12 +99,12 @@ | |
| ) | ||
| ) | ||
|
|
||
| return hasFeature && hasTrialDays && hasPrice && !alreadyActivated | ||
| return hasRequirement && hasTrialDays && hasPrice && !alreadyActivated | ||
| }) | ||
| .sort((a, b) => (b.plan?.priority ?? 0) - (a.plan?.priority ?? 0)) | ||
|
|
||
| return plansWithFeature[0] || null | ||
| }, [feature, isAvailable, plansData?.result?.plans, activatedSubscriptions]) | ||
| }, [feature, b2bAppId, isAvailable, plansData?.result?.plans, activatedSubscriptions]) | ||
|
|
||
| const TryTrialButton = useMemo(() => { | ||
| const currencyCode = bestPlanWithFeature?.prices?.[0]?.currencyCode | ||
|
|
@@ -127,11 +146,11 @@ | |
|
|
||
| const buttonText = hasActivatedAnyTrial ? ViewPlansButton : TryTrialButton | ||
|
|
||
| const buttonAction = !feature || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial | ||
| const buttonAction = (!feature && !b2bAppId) || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial | ||
|
|
||
| const tooltipTitle = ( | ||
| <Space size={12} direction='vertical'> | ||
| <Typography.Text size='small'>{hasActivatedAnyTrial ? UpgradePlanMessage : FeatureLockedMessage}</Typography.Text> | ||
| <Typography.Text size='small'>{hasActivatedAnyTrial && !isLoading ? UpgradePlanMessage : FeatureLockedMessage}</Typography.Text> | ||
|
Comment on lines
+149
to
+153
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use the "view plans" CTA when no matching trial exists. After the new 💡 Proposed fix- const buttonText = hasActivatedAnyTrial ? ViewPlansButton : TryTrialButton
-
- const buttonAction = (!feature && !b2bAppId) || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial
+ const canStartTrial = Boolean(bestPlanWithFeature) && !hasActivatedAnyTrial
+ const buttonText = canStartTrial ? TryTrialButton : ViewPlansButton
+
+ const buttonAction = canStartTrial ? handleActivateTrial : handleViewPlans🤖 Prompt for AI Agents |
||
| <Button | ||
| type='accent' | ||
| size='medium' | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,47 +5,61 @@ | |||||||||||||||||
|
|
||||||||||||||||||
| interface SubscriptionFeatureGuardProps extends Omit<NoSubscriptionTooltipProps, 'children'> { | ||||||||||||||||||
| children: React.ReactElement | ||||||||||||||||||
| feature: NoSubscriptionTooltipProps['feature'] | ||||||||||||||||||
| feature?: NoSubscriptionTooltipProps['feature'] | ||||||||||||||||||
| fallback: React.ReactElement | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** | ||||||||||||||||||
| * Component that conditionally renders content based on subscription feature availability. | ||||||||||||||||||
| * | ||||||||||||||||||
| * If the organization has access to the specified feature(s), renders the children. | ||||||||||||||||||
| * Component that conditionally renders content based on subscription feature or miniapp availability. | ||||||||||||||||||
| * | ||||||||||||||||||
| * If the organization has access to the specified feature(s) or miniapp, renders the children. | ||||||||||||||||||
| * Otherwise, renders the fallback element wrapped in a NoSubscriptionTooltip. | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param children - Element to render when feature is available | ||||||||||||||||||
| * @param feature - Feature name or array of feature names to check | ||||||||||||||||||
| * @param fallback - Element to render when feature is not available (will be wrapped in tooltip) | ||||||||||||||||||
| * | ||||||||||||||||||
| * @param children - Element to render when feature/miniapp is available | ||||||||||||||||||
| * @param feature - Feature name or array of feature names to check (optional if b2bAppId is provided) | ||||||||||||||||||
| * @param b2bAppId - Miniapp ID to check availability for (optional if feature is provided) | ||||||||||||||||||
| * @param fallback - Element to render when feature/miniapp is not available (will be wrapped in tooltip) | ||||||||||||||||||
| * @param tooltipProps - Additional props passed to NoSubscriptionTooltip | ||||||||||||||||||
| * | ||||||||||||||||||
| * | ||||||||||||||||||
| * @example | ||||||||||||||||||
| * // Feature-based guard | ||||||||||||||||||
| * <SubscriptionGuardWithTooltip | ||||||||||||||||||
| * feature="analytics" | ||||||||||||||||||
| * fallback={<Button disabled>Analytics</Button>} | ||||||||||||||||||
| * > | ||||||||||||||||||
| * <Button>Analytics</Button> | ||||||||||||||||||
| * </SubscriptionGuardWithTooltip> | ||||||||||||||||||
| * | ||||||||||||||||||
| * @example | ||||||||||||||||||
| * // Miniapp-based guard | ||||||||||||||||||
| * <SubscriptionGuardWithTooltip | ||||||||||||||||||
| * b2bAppId="miniapp-id" | ||||||||||||||||||
| * fallback={<Button disabled>Connect</Button>} | ||||||||||||||||||
| * > | ||||||||||||||||||
| * <Button>Connect</Button> | ||||||||||||||||||
| * </SubscriptionGuardWithTooltip> | ||||||||||||||||||
| */ | ||||||||||||||||||
| export const SubscriptionGuardWithTooltip: React.FC<SubscriptionFeatureGuardProps> = ({ | ||||||||||||||||||
| children, | ||||||||||||||||||
| feature, | ||||||||||||||||||
| fallback, | ||||||||||||||||||
| b2bAppId, | ||||||||||||||||||
| ...tooltipProps | ||||||||||||||||||
| }) => { | ||||||||||||||||||
| const { isFeatureAvailable } = useOrganizationSubscription() | ||||||||||||||||||
| const { isFeatureAvailable, isB2BAppEnabled } = useOrganizationSubscription() | ||||||||||||||||||
|
|
||||||||||||||||||
| const isAppAvailableForTariff = b2bAppId ? isB2BAppEnabled(b2bAppId) : true | ||||||||||||||||||
|
|
||||||||||||||||||
| const hasFeature = Array.isArray(feature) | ||||||||||||||||||
| ? feature.every(f => isFeatureAvailable(f)) | ||||||||||||||||||
| ? feature.every(isFeatureAvailable) | ||||||||||||||||||
|
Check failure on line 54 in apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx
|
||||||||||||||||||
| : isFeatureAvailable(feature) | ||||||||||||||||||
|
Comment on lines
53
to
55
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't call
💡 Proposed fix- const hasFeature = Array.isArray(feature)
- ? feature.every(f => isFeatureAvailable(f))
- : isFeatureAvailable(feature)
+ const hasFeature = feature
+ ? Array.isArray(feature)
+ ? feature.every(f => isFeatureAvailable(f))
+ : isFeatureAvailable(feature)
+ : false📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| if (hasFeature) { | ||||||||||||||||||
| if (hasFeature || isAppAvailableForTariff) { | ||||||||||||||||||
|
||||||||||||||||||
| return children | ||||||||||||||||||
| } | ||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
|
|
||||||||||||||||||
| return ( | ||||||||||||||||||
| <NoSubscriptionTooltip feature={feature} {...tooltipProps}> | ||||||||||||||||||
| <NoSubscriptionTooltip feature={feature} b2bAppId={b2bAppId} {...tooltipProps}> | ||||||||||||||||||
| {fallback} | ||||||||||||||||||
| </NoSubscriptionTooltip> | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -170,7 +170,7 @@ | |||||||||||||||||
|
|
||||||||||||||||||
| const { isAuthenticated, isLoading } = useAuth() | ||||||||||||||||||
| const { employee, organization } = useOrganization() | ||||||||||||||||||
| const { hasSubscription } = useOrganizationSubscription() | ||||||||||||||||||
| const { hasSubscription, isB2BAppEnabled } = useOrganizationSubscription() | ||||||||||||||||||
| const disabled = !employee || !hasSubscription | ||||||||||||||||||
| const { isCollapsed } = useLayoutContext() | ||||||||||||||||||
| const { wrapElementIntoNoOrganizationToolTip } = useNoOrganizationToolTip() | ||||||||||||||||||
|
|
@@ -403,6 +403,19 @@ | |||||||||||||||||
| // not a ReDoS issue: running on end user browser | ||||||||||||||||||
| // nosemgrep: javascript.lang.security.audit.detect-non-literal-regexp.detect-non-literal-regexp | ||||||||||||||||||
| const miniAppsPattern = new RegExp(`/miniapps/${app.id}/.+`) | ||||||||||||||||||
| const isAppAvailable = isB2BAppEnabled(app.id) | ||||||||||||||||||
|
|
||||||||||||||||||
| const miniappTooltip = ({ element, placement }) => ( | ||||||||||||||||||
| <NoSubscriptionTooltip b2bAppId={app.id} children={element} placement={placement} /> | ||||||||||||||||||
|
Check warning on line 409 in apps/condo/pages/_app.tsx
|
||||||||||||||||||
| ) | ||||||||||||||||||
|
Comment on lines
+408
to
+410
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid passing children using the The static analysis tool flagged this pattern. Use standard JSX children syntax for clarity. 🐛 Proposed fix const miniappTooltip = ({ element, placement }) => (
- <NoSubscriptionTooltip b2bAppId={app.id} children={element} placement={placement} />
+ <NoSubscriptionTooltip b2bAppId={app.id} placement={placement}>
+ {element}
+ </NoSubscriptionTooltip>
)📝 Committable suggestion
Suggested change
🧰 Tools🪛 Biome (2.4.6)[error] 409-409: Avoid passing children using a prop (lint/correctness/noChildrenProp) 🤖 Prompt for AI Agents |
||||||||||||||||||
|
|
||||||||||||||||||
| let tooltipDecorator = null | ||||||||||||||||||
| if (disabled) { | ||||||||||||||||||
| tooltipDecorator = wrapElementIntoNoOrganizationToolTip | ||||||||||||||||||
| } else if (!isAppAvailable) { | ||||||||||||||||||
| tooltipDecorator = miniappTooltip | ||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This tooltip only for miniapps?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because we use it in appsByCategories |
||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| return <MenuItem | ||||||||||||||||||
| id={`menu-item-app-${app.id}`} | ||||||||||||||||||
| key={`menu-item-app-${app.id}`} | ||||||||||||||||||
|
|
@@ -412,7 +425,7 @@ | |||||||||||||||||
| labelRaw | ||||||||||||||||||
| disabled={disabled} | ||||||||||||||||||
| isCollapsed={isCollapsed} | ||||||||||||||||||
| toolTipDecorator={disabled ? wrapElementIntoNoOrganizationToolTip : null} | ||||||||||||||||||
| toolTipDecorator={tooltipDecorator} | ||||||||||||||||||
| excludePaths={[miniAppsPattern]} | ||||||||||||||||||
| /> | ||||||||||||||||||
| })} | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid passing children using the
childrenprop; use JSX children syntax instead.The static analysis tool flagged this as an error. Using the
childrenprop directly is discouraged—prefer the standard JSX children pattern for clarity and to avoid potential issues.🐛 Proposed fix
<SubscriptionGuardWithTooltip b2bAppId={id} fallback={ - <span><Button {...buttonProps} disabled children={UnavailableForTariffMessage}/></span> + <span><Button {...buttonProps} disabled>{UnavailableForTariffMessage}</Button></span> }> <Button {...buttonProps}/> </SubscriptionGuardWithTooltip>📝 Committable suggestion
🤖 Prompt for AI Agents