Skip to content

Conversation

@catsiller
Copy link
Contributor

@catsiller catsiller commented Dec 8, 2025

Description

When a user has an ETAC limited account and they navigate to the Explore page, they see a note at the top of the page stating “Role-based access filtering applied” with an eye slash icon (icon on left side of text)

Things to check:

  1. light mode has background of #F8EEFD with white text
  2. dark mode has background of #472E54 with black text
  3. different than design mocks in Jira ticket - Badge should be above object information panel on right hand side of page
  4. badge should have amount of spacing that is equal to that between the object name and object description

Note
This only applies to user or read only without access to all environments.

Motivation and Context

Resolves https://specterops.atlassian.net/browse/BED-6935

Why is this change required? What problem does it solve?

How Has This Been Tested?

Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.

Screenshots (optional):

Types of changes

  • Chore (a change that does not modify the application functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Database Migrations

Checklist:

Summary by CodeRabbit

  • New Features
    • Added a visual badge indicator that displays when role-based access filtering is applied, providing a clear signal that access restrictions are in effect.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025

Walkthrough

This change enhances the EntityInfoPanel component by adding role-based access filtering visualization. The component now fetches the current user's roles and determines if ETAC filtering applies, conditionally displaying a Badge indicator showing when role-based access filtering is active.

Changes

Cohort / File(s) Change Summary
ETAC role-based filtering badge
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx
Added imports for Badge, FontAwesomeIcon, faEyeSlash, and hooks (useSelf, useListDisplayRoles); implemented role-based filtering logic to compute when ETAC filtering applies; conditionally renders a new Badge component with eye-slash icon when filtering is active and not on privilege zones page.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Review the role computation logic (userRoleId, selectedETACEnabledRole, roleBasedFiltering conditions)
  • Verify conditional rendering placement and styling alignment with existing panel structure
  • Confirm hook dependencies and data flow correctness

Suggested labels

enhancement, user interface

Suggested reviewers

  • maffkipp

Poem

🐰 A badge hops in, eye-slash in tow,
Role-based filtering now on show!
When ETAC reigns and zones don't play,
This little icon marks the way. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: explore badge limited view BED-6935' clearly summarizes the main change: adding a feature for displaying a badge on the explore page for limited view users.
Description check ✅ Passed The description covers the main objective (displaying a role-based filtering badge), includes visual specifications (colors for light/dark modes, spacing requirements), scope limitations (ETAC users without all environments access), and references the Jira ticket. However, testing details and checklist items remain incomplete/unchecked.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch BED-6935--explore-badge-limited-view

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@catsiller catsiller changed the title Bed 6935 explore badge limited view feat: explore badge limited view BED-6935 Dec 8, 2025
@catsiller catsiller force-pushed the BED-6935--explore-badge-limited-view branch 2 times, most recently from 5b2e589 to 4476524 Compare December 8, 2025 19:34
@catsiller catsiller force-pushed the BED-6935--explore-badge-limited-view branch from b592fdd to b8c45c0 Compare December 8, 2025 21:56
@catsiller catsiller marked this pull request as ready for review December 9, 2025 19:32
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d6488e and 377827d.

📒 Files selected for processing (1)
  • packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-11T18:25:23.685Z
Learnt from: TheNando
Repo: SpecterOps/BloodHound PR: 1737
File: packages/javascript/bh-shared-ui/src/components/StatusIndicator.tsx:23-35
Timestamp: 2025-08-11T18:25:23.685Z
Learning: In the BloodHound bh-shared-ui package, React is automatically injected into scope by the framework, so explicit React imports are not required when using React.FC or other React types in component files.

Applied to files:

  • packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx
🧬 Code graph analysis (1)
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx (4)
packages/javascript/bh-shared-ui/src/hooks/useSelf/useSelf.tsx (1)
  • useSelf (22-39)
packages/javascript/bh-shared-ui/src/hooks/useListDisplayRoles/useListDisplayRoles.tsx (1)
  • useListDisplayRoles (20-25)
packages/javascript/bh-shared-ui/src/utils/roles.ts (1)
  • isETACRole (34-37)
packages/javascript/bh-shared-ui/src/utils/theme.ts (1)
  • cn (41-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Build BloodHound Container Image / Build and Package Container
  • GitHub Check: run-tests
🔇 Additional comments (1)
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx (1)

16-21: New imports look consistent with usage in this component

The added imports for Badge, FontAwesome, useSelf, useListDisplayRoles, and isETACRole are all exercised later in the file and match the way these utilities/components are typically wired, so no issues here from a dependency/usage perspective.

Also applies to: 25-25

Comment on lines +52 to +58
const getSelfQuery = useSelf();
const getRolesQuery = useListDisplayRoles();
const roles = getRolesQuery.data;
const userRoleId = getSelfQuery?.data?.roles.map((item: any) => item.id);
const selectedETACEnabledRole = isETACRole(Number(userRoleId), roles);
const roleBasedFiltering: boolean = getSelfQuery?.data?.all_environments === false && selectedETACEnabledRole;

Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for isETACRole function definition
rg "function isETACRole|const isETACRole|export.*isETACRole" -A 5

Repository: SpecterOps/BloodHound

Length of output: 706


🏁 Script executed:

#!/bin/bash
# Find the EntityInfoPanel.tsx file and read the relevant section
fd "EntityInfoPanel.tsx" --exec cat -n {} \; | head -100

Repository: SpecterOps/BloodHound

Length of output: 5185


🏁 Script executed:

#!/bin/bash
# Search for useListDisplayRoles and useSelf hooks to understand data structure
rg "useListDisplayRoles|useSelf" -A 3 --max-count 30

Repository: SpecterOps/BloodHound

Length of output: 13398


🏁 Script executed:

#!/bin/bash
# Check if multi-role users are actually supported in the system
# Look for role type definitions and usage patterns
rg "roles\s*:" packages/javascript/bh-shared-ui/src -A 2 | grep -E "(Role\[\]|Array<Role>|roles\?)" | head -20

Repository: SpecterOps/BloodHound

Length of output: 175


🏁 Script executed:

#!/bin/bash
# Check if there are any tests for ETAC role detection with multiple roles
fd "test|spec" -t f -name "*.test.tsx" -o -name "*.spec.tsx" | xargs rg -l "isETACRole|selectedETACEnabledRole" 2>/dev/null

Repository: SpecterOps/BloodHound

Length of output: 607


🏁 Script executed:

#!/bin/bash
# Look at CreateUserForm and UpdateUserForm to see how they handle multi-role scenarios
rg -A 10 "isETACRole" packages/javascript/bh-shared-ui/src/components/CreateUserForm/CreateUserForm.tsx packages/javascript/bh-shared-ui/src/components/UpdateUserForm/UpdateUserForm.tsx

Repository: SpecterOps/BloodHound

Length of output: 5730


Role/ETAC detection fails for multi‑role users: .map + Number() coercion produces NaN

The current code at lines 55–56:

const userRoleId = getSelfQuery?.data?.roles.map((item: any) => item.id);
const selectedETACEnabledRole = isETACRole(Number(userRoleId), roles);

userRoleId is an array, but isETACRole expects a single number | undefined. Using Number() on an array:

  • Works for single-role users: Number([42])42
  • Fails for multi-role users: Number([1, 2])NaN, causing isETACRole to always return false

This breaks the role-based filtering badge for users with multiple roles.

Check all user role IDs against ETAC roles instead:

-const userRoleId = getSelfQuery?.data?.roles.map((item: any) => item.id);
-const selectedETACEnabledRole = isETACRole(Number(userRoleId), roles);
+const userRoleIds = getSelfQuery?.data?.roles?.map((item: any) => item.id as number) ?? [];
+const selectedETACEnabledRole =
+    roles != null && userRoleIds.some((roleId) => isETACRole(roleId, roles));
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx
around lines 52–58, the code maps user roles to an array then coerces that array
to a Number which yields NaN for multi-role users; instead extract the array of
role IDs safely (handle undefined), then set selectedETACEnabledRole by checking
if any of the user's role IDs match ETAC roles (e.g., use .some over the ids and
call isETACRole for each id), and ensure roleBasedFiltering uses that boolean;
also keep types correct (number | undefined) and guard when getRolesQuery.data
or getSelfQuery.data is missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with coderabbit here that we probably want some additional handling here for the userRoleId array. Even though our API only returns a single value in the roles array currently, that is not necessarily something we can depend on for the future.

Since this is all logic we might want to use again in other places (is role-based filtering enabled for a user), it also might be worthwhile to extract out into a custom hook!

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for similar patterns where useSelf and useListDisplayRoles are used together for ETAC role checking
rg -n "useSelf\(\)" -A 10 -B 2 | rg -A 10 -B 2 "useListDisplayRoles|isETACRole" | head -80

Length of output: 1799


🏁 Script executed:

#!/bin/bash
# Check the structure of existing custom hooks for patterns
fd "use.*\.(ts|tsx)" packages/javascript/bh-shared-ui/src/hooks/ --type f | head -10

Length of output: 761


🏁 Script executed:

#!/bin/bash
# Look at a couple of example hooks to understand the pattern
cat packages/javascript/bh-shared-ui/src/hooks/useBloodHoundUsers/useBloodHoundUsers.ts | head -50

Length of output: 115


🏁 Script executed:

#!/bin/bash
# Check the actual useSelf implementation being used in EntityInfoPanel
cat packages/javascript/bh-shared-ui/src/hooks/useBloodHoundUsers.ts

Length of output: 1531


🏁 Script executed:

#!/bin/bash
# Look for the getSelf function to understand the data structure
rg "function getSelf|const getSelf|export.*getSelf" -A 5 packages/javascript/bh-shared-ui/src

Length of output: 5532


🏁 Script executed:

#!/bin/bash
# Check existing hook patterns in the hooks directory
ls -la packages/javascript/bh-shared-ui/src/hooks/ | grep -E "^d"

Length of output: 1452


🏁 Script executed:

#!/bin/bash
# Look at a simpler hook example to understand the pattern
cat packages/javascript/bh-shared-ui/src/hooks/useAvailableEnvironments/useAvailableEnvironments.tsx

Length of output: 2731


@maffkipp, great suggestion! Extracting this into a custom hook will make it reusable and ensure consistent behavior across the application. Here's a complete implementation:

Create the Custom Hook

File: packages/javascript/bh-shared-ui/src/hooks/useRoleBasedFiltering/useRoleBasedFiltering.tsx

// Copyright 2025 Specter Ops, Inc.
//
// Licensed under the Apache License, Version 2.0
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0

import { useSelf } from '../useBloodHoundUsers';
import { useListDisplayRoles } from '../useListDisplayRoles/useListDisplayRoles';
import { isETACRole } from '../../utils/roles';

/**
 * Hook to determine if role-based filtering is enabled for the current user.
 * Role-based filtering is enabled when:
 * 1. The user does not have access to all environments (all_environments is false)
 * 2. At least one of the user's roles is an ETAC role
 * 
 * @returns boolean indicating whether role-based filtering is active
 */
export const useRoleBasedFiltering = (): boolean => {
    const getSelfQuery = useSelf();
    const getRolesQuery = useListDisplayRoles();

    // Extract user role IDs safely, handling undefined/null cases
    const userRoleIds = getSelfQuery?.data?.roles?.map((role: any) => role.id as number) ?? [];
    const allEnvironments = getSelfQuery?.data?.all_environments ?? true;
    const roles = getRolesQuery.data;

    // Check if any of the user's roles is an ETAC role
    const hasETACRole = roles != null && userRoleIds.some((roleId) => isETACRole(roleId, roles));

    // Role-based filtering is enabled if user doesn't have all_environments AND has an ETAC role
    return !allEnvironments && hasETACRole;
};

Export the Hook

File: packages/javascript/bh-shared-ui/src/hooks/useRoleBasedFiltering/index.ts

// Copyright 2025 Specter Ops, Inc.
//
// Licensed under the Apache License, Version 2.0
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0

export * from './useRoleBasedFiltering';

Refactor EntityInfoPanel

File: packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx

// Copyright 2025 Specter Ops, Inc.
//
// Licensed under the Apache License, Version 2.0
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
//     http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0
import { Badge } from '@bloodhoundenterprise/doodleui';
import { faEyeSlash } from '@fortawesome/free-solid-svg-icons';
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
import React, { HTMLProps } from 'react';
import { useRoleBasedFiltering } from '../../hooks/useRoleBasedFiltering';
import { privilegeZonesPath } from '../../routes';
import { SelectedNode } from '../../types';
import { EntityInfoDataTableProps, NoEntitySelectedHeader, NoEntitySelectedMessage, cn } from '../../utils';
import { ObjectInfoPanelContextProvider } from '../../views/Explore/providers/ObjectInfoPanelProvider';
import EntityInfoContent from './EntityInfoContent';
import Header from './EntityInfoHeader';

export type EntityTables = {
    sectionProps: EntityInfoDataTableProps;
    TableComponent: React.FC<EntityInfoDataTableProps>;
}[];

interface EntityInfoPanelProps {
    DataTable: React.FC<EntityInfoDataTableProps>;
    selectedNode?: SelectedNode | null;
    className?: HTMLProps<HTMLDivElement>['className'];
    additionalTables?: EntityTables;
    priorityTables?: EntityTables;
}

const EntityInfoPanel: React.FC<EntityInfoPanelProps> = ({
    selectedNode,
    className,
    additionalTables,
    priorityTables,
    DataTable,
}) => {
    const isPrivilegeZonesPage = location.pathname.includes(`/${privilegeZonesPath}`);
    const roleBasedFiltering = useRoleBasedFiltering();

    return (
        <div
            className={cn(
                'flex flex-col pointer-events-none overflow-y-hidden h-full min-w-[400px] w-[400px] max-w-[400px]',
                className
            )}
            data-testid='explore_entity-information-panel'>
            {!isPrivilegeZonesPage && roleBasedFiltering && (
                <Badge
                    data-testid='explore_entity-information-panel-badge-etac-filtering'
                    className='!justify-start text-sm !text-neutral-dark-1 bg-[#F8EEFD] dark:bg-[#472E54] dark:!text-neutral-light-1 border-0 mb-2'
                    icon={<FontAwesomeIcon icon={faEyeSlash} />}
                    label='&nbsp; Role-based access filtering applied'
                />
            )}
            <div className='bg-neutral-2 pointer-events-auto rounded'>
                <Header name={selectedNode?.name || NoEntitySelectedHeader} nodeType={selectedNode?.type} />
            </div>
            <div className='bg-neutral-2 mt-2 overflow-x-hidden overflow-y-auto py-1 px-4 pointer-events-auto rounded'>
                {selectedNode ? (
                    <EntityInfoContent
                        DataTable={DataTable}
                        id={selectedNode.id}
                        nodeType={selectedNode.type}
                        databaseId={selectedNode.graphId}
                        priorityTables={priorityTables}
                        additionalTables={additionalTables}
                    />
                ) : (
                    <p className='text-sm'>
                        {isPrivilegeZonesPage
                            ? 'Select an object to view the associated information'
                            : NoEntitySelectedMessage}
                    </p>
                )}
            </div>
        </div>
    );
};

const WrappedEntityInfoPanel: React.FC<EntityInfoPanelProps> = (props) => (
    <ObjectInfoPanelContextProvider>
        <EntityInfoPanel {...props} />
    </ObjectInfoPanelContextProvider>
);

export default WrappedEntityInfoPanel;

Key Benefits

  1. Future-proof: Handles multi-role scenarios correctly using .some() instead of coercing an array to a number
  2. Reusable: Other components can now use useRoleBasedFiltering() wherever needed
  3. Type-safe: Properly typed role IDs as number instead of any
  4. Defensive: Handles undefined/null data gracefully with nullish coalescing and optional chaining
  5. Cleaner: EntityInfoPanel is now much simpler and focused on rendering
  6. Documented: JSDoc explains when role-based filtering is enabled

The hook encapsulates all the logic for determining if role-based filtering should be active, making it easy to reuse this logic anywhere else in the application where you need to check if the current user has ETAC-based filtering enabled.


🧬 Code Graph Analysis Results ### File: `packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx`
  • File context: The panel references user role-based filtering logic, uses hooks useSelf and useListDisplayRoles, and relies on isETACRole to determine role-based filtering. The user comment suggests factoring out role-based logic into a custom hook to reuse elsewhere.

  • Relevant snippets to understand the file and address the user comment:

  1. File: packages/javascript/bh-shared-ui/src/hooks/useSelf/useSelf.tsx
    Line range: 22-39
export const useSelf = () => {
    const getSelfId = useQuery(['getSelf'], ({ signal }) => getSelf({ signal }), {
        cacheTime: Number.POSITIVE_INFINITY,
        select: (data) => {
            return data.id;
        },
    });

    const getSelfRoles = useQuery(['getSelf'], ({ signal }) => getSelf({ signal }), {
        cacheTime: Number.POSITIVE_INFINITY,
        select: (data) => {
            const userRoles = data?.roles?.map((role: any) => role.name) || [];
            return userRoles;
        },
    });

    return { getSelfId, getSelfRoles };
};
  1. File: packages/javascript/bh-shared-ui/src/hooks/useListDisplayRoles/useListDisplayRoles.tsx
    Line range: 20-25
export const useListDisplayRoles = () => {
    return useQuery({
        queryKey: ['getRoles'],
        queryFn: ({ signal }) => apiClient.getRoles({ signal }).then((res) => res.data?.data?.roles),
    });
};
  1. File: packages/javascript/bh-shared-ui/src/utils/roles.ts
    Line range: 34-37
export const isETACRole = (roleId: number | undefined, roles?: Role[]): boolean => {
    const matchingRole = roles?.find((role) => roleId === role.id)?.name;
    return !!(matchingRole && ETAC_ROLES.some((role) => role === matchingRole));
};
  1. File: packages/javascript/bh-shared-ui/src/types.ts
    Line range: 101-106
export type SelectedNode = {
    id: string;
    type: EntityKinds;
    name: string;
    graphId?: string;
};
  1. File: packages/javascript/bh-shared-ui/src/views/Explore/providers/ObjectInfoPanelProvider/ObjectInfoPanelContextProvider.tsx
    Line range: 22-32
export function ObjectInfoPanelContextProvider({ children }: ObjectInfoPanelContextProviderProps) {
    const [isObjectInfoPanelOpen, setIsObjectInfoPanelOpen] = useState<boolean>(true);
    const value = useMemo(
        () => ({
            isObjectInfoPanelOpen,
            setIsObjectInfoPanelOpen,
        }),
        [isObjectInfoPanelOpen]
    );
    return <ObjectInfoPanelContext.Provider value={value}>{children}</ObjectInfoPanelContext.Provider>;
}

🧠 Learnings used
Learnt from: jvacca-specterops
Repo: SpecterOps/BloodHound PR: 1823
File: packages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/QuerySearchFilter.tsx:110-116
Timestamp: 2025-09-08T20:04:09.070Z
Learning: In BloodHound's TypeScript codebase, jvacca-specterops considers runtime guards for type assertions unnecessary when the UI state already prevents the problematic scenario (e.g., disabled buttons when required data is unavailable), preferring to rely on the existing disabled state logic rather than adding additional runtime checks.

Comment on lines +66 to +73
{!isPrivilegeZonesPage && roleBasedFiltering && (
<Badge
data-testid='explore_entity-information-panel-badge-etac-filtering'
className='!justify-start text-sm !text-neutral-dark-1 bg-[#F8EEFD] dark:bg-[#472E54] dark:!text-neutral-light-1 border-0 mb-2'
icon={<FontAwesomeIcon icon={faEyeSlash} />}
label='&nbsp; Role-based access filtering applied'
/>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Badge styling and label: PR-spec color mismatch and HTML entity in label

Two small issues in the new Badge:

<Badge
    data-testid='explore_entity-information-panel-badge-etac-filtering'
    className='!justify-start text-sm !text-neutral-dark-1 bg-[#F8EEFD] dark:bg-[#472E54] dark:!text-neutral-light-1 border-0 mb-2'
    icon={<FontAwesomeIcon icon={faEyeSlash} />}
    label='&nbsp; Role-based access filtering applied'
/>
  1. Color spec vs implementation

    • PR objective calls out:
      • Light mode: background #F8EEFD with white text.
      • Dark mode: background #472E54 with black text.
    • Current classes use !text-neutral-dark-1 in light mode and dark:!text-neutral-light-1 in dark mode, which is the opposite of the requested contrast. This may be intentional for accessibility, but it diverges from the stated requirement; worth confirming with design and adjusting the text utility classes if they do want literal white/black.
  2. &nbsp; will likely render literally

    • Passing '&nbsp; Role-based access filtering applied' as a string means React (and likely the Badge component) will treat it as plain text, so users may see the &nbsp; characters instead of a leading non‑breaking space.
    • If extra spacing after the icon is needed, it’s safer to rely on layout (e.g., gap/margin utilities or Badge’s built‑in icon spacing) and keep the label clean:
-    label='&nbsp; Role-based access filtering applied'
+    label='Role-based access filtering applied'

I’d recommend updating the text color utilities to match the agreed spec (or updating the spec if the current contrast is preferred) and dropping the HTML entity from the label.

🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/EntityInfo/EntityInfoPanel.tsx
around lines 66-73, the Badge uses the wrong text color utilities (light mode
should render white text on #F8EEFD and dark mode should render black text on
#472E54) and the label contains a literal HTML entity '&nbsp;' which will be
rendered as text; to fix, swap the text utility classes so light mode forces
white text and dark mode forces black text to match the PR spec (or confirm with
design if the current contrast was intentional) and remove the '&nbsp;' from the
label, instead spacing the icon and label via CSS/layout (use the Badge/icon
spacing or Tailwind gap/margin utilities).

{!isPrivilegeZonesPage && roleBasedFiltering && (
<Badge
data-testid='explore_entity-information-panel-badge-etac-filtering'
className='!justify-start text-sm !text-neutral-dark-1 bg-[#F8EEFD] dark:bg-[#472E54] dark:!text-neutral-light-1 border-0 mb-2'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to add styles to this component without using ! on all these styles -- definitely think we should do some digging into why if they can't be applied normally, since in doodle it looks like we are merging in styles from the className prop as expected

Comment on lines +52 to +58
const getSelfQuery = useSelf();
const getRolesQuery = useListDisplayRoles();
const roles = getRolesQuery.data;
const userRoleId = getSelfQuery?.data?.roles.map((item: any) => item.id);
const selectedETACEnabledRole = isETACRole(Number(userRoleId), roles);
const roleBasedFiltering: boolean = getSelfQuery?.data?.all_environments === false && selectedETACEnabledRole;

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with coderabbit here that we probably want some additional handling here for the userRoleId array. Even though our API only returns a single value in the roles array currently, that is not necessarily something we can depend on for the future.

Since this is all logic we might want to use again in other places (is role-based filtering enabled for a user), it also might be worthwhile to extract out into a custom hook!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants