Skip to content
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

fix: display beta tag with css #14478

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

fix: display beta tag with css #14478

wants to merge 8 commits into from

Conversation

standeren
Copy link
Contributor

@standeren standeren commented Jan 21, 2025

Description

Display beta tag in header menu items with css.

See examples of how it will look like:

Skjermbilde 2025-01-21 kl  17 05 50
Skjermbilde 2025-01-21 kl  17 05 53
Skjermbilde 2025-01-21 kl  17 06 05
Skjermbilde 2025-01-21 kl  17 06 08

⚠️ The actual menu item for the content menu is not added in this PR ⚠️

Related Issue(s)

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)

Summary by CodeRabbit

  • Removal

    • Removed StudioBetaTag component and related files
    • Eliminated explicit "Beta" tag rendering
  • New Features

    • Introduced isBeta prop for menu items
    • Added CSS class for beta-tagged items
  • Style Changes

    • Updated navigation menu styling to conditionally display beta status
    • Added new CSS class for beta-tagged elements
  • Testing

    • Updated test cases to reflect new beta tagging approach

@standeren standeren added kind/bug Used when there is a defect / something is not working as it should. skip-releasenotes Issues that do not make sense to list in our release notes frontend team/studio-domain1 skip-documentation Issues where updating documentation is not relevant labels Jan 21, 2025
Copy link
Contributor

coderabbitai bot commented Jan 21, 2025

📝 Walkthrough

Walkthrough

The pull request involves a comprehensive refactoring of the beta tag functionality across multiple frontend components. The primary changes include removing the StudioBetaTag component, introducing an isBeta prop to various components, and replacing the component-based beta tag with a CSS-based approach. This modification simplifies the beta status representation by using a conditional class instead of a separate component, affecting navigation menus, header links, and other UI elements.

Changes

File Change Summary
frontend/libs/studio-components/src/style/studioBetaTag.module.css Added .isBeta:after CSS class for rendering beta status
frontend/libs/studio-components/src/index.ts Exported studioBetaTagClasses
frontend/app-development/layout/PageHeader/LargeNavigationMenu/... Removed .betaTag CSS class, updated component to use isBeta prop
frontend/libs/studio-components/src/components/StudioPageHeader/... Added isBeta prop to StudioPageHeaderHeaderLink
frontend/app-development/features/overview/components/Navigation/... Updated to use isBeta class instead of StudioBetaTag
frontend/app-development/layout/PageHeader/SmallHeaderMenu/... Replaced StudioBetaTag with studioBetaTagClasses.isBeta

Possibly related PRs

Suggested labels

area/ui-editor

Suggested reviewers

  • Jondyr
  • ErlingHauan

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31620e0 and cee11b6.

📒 Files selected for processing (1)
  • frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the solution/studio/designer Issues related to the Altinn Studio Designer solution. label Jan 21, 2025
@standeren standeren linked an issue Jan 21, 2025 that may be closed by this pull request
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

🧹 Nitpick comments (1)
frontend/libs/studio-components/src/components/StudioPageHeader/common.module.css (1)

47-53: Consider adding spacing and adjusting padding for better visual harmony.

Good use of design system variables and semantic colors. However, consider these improvements:

  1. Add margin/gap between the beta tag and adjacent content
  2. Use different padding values for horizontal vs vertical to make the tag more compact
 .isBeta {
   background-color: var(--fds-semantic-surface-info-subtle);
   color: var(--fds-semantic-text-neutral-default);
   border-radius: var(--fds-border_radius-large);
-  padding: var(--fds-spacing-1);
+  padding: var(--fds-spacing-0) var(--fds-spacing-1);
+  margin-left: var(--fds-spacing-1);
   font-size: var(--fds-sizing-3);
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e91be7 and 894c94c.

📒 Files selected for processing (9)
  • frontend/app-development/enums/HeaderMenuItemKey.ts (1 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.module.css (1 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (3 hunks)
  • frontend/language/src/nb.json (1 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/common.module.css (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: CodeQL
  • GitHub Check: Testing
  • GitHub Check: Build environment and run e2e test
🔇 Additional comments (9)
frontend/libs/studio-components/src/components/StudioPageHeader/common.module.css (2)

42-45: LGTM! Clean flexbox implementation.

The .betaContainer class follows flexbox best practices with appropriate alignment properties for consistent beta tag placement.


42-53: Verify integration with header menu items.

The CSS implementation looks good for the beta tag styling. To ensure proper integration, verify that:

  1. The beta tag appears correctly aligned with different header menu item lengths
  2. The styling remains consistent across different viewport sizes
  3. The contrast ratio meets accessibility standards with the chosen semantic colors
✅ Verification successful

Beta tag integration is properly implemented and tested ✓

The implementation shows:

  • Correct styling application through component composition
  • Comprehensive test coverage including accessibility
  • Proper handling of beta tags in both large and small menu items
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of these classes to verify proper integration
rg -l "betaContainer|isBeta" frontend/

Length of output: 1190


Script:

#!/bin/bash
# Check implementation of StudioBetaTag and its usage in header components
echo "=== StudioBetaTag Implementation ==="
cat frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx

echo -e "\n=== StudioPageHeaderHeaderLink Usage ==="
cat frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx

echo -e "\n=== Test Coverage ==="
cat frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.test.tsx

Length of output: 5198

frontend/app-development/enums/HeaderMenuItemKey.ts (1)

8-8: LGTM! The new enum entry follows consistent patterns.

The addition of ContentLibrary maintains the established naming convention and value pattern, appropriately placed before the None entry.

frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx (1)

12-12: LGTM! Story simplified to match new implementation.

The removal of size controls and props aligns with the component's new CSS-based styling approach.

frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (1)

5-7: LGTM! Clean type definition with flexible styling support.

The new type with optional className prop provides good flexibility for custom styling needs.

frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx (1)

12-12: LGTM! Well-typed optional beta property.

The isBeta prop is properly typed as optional, maintaining type safety.

frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (1)

5-5: LGTM! Clean implementation of beta tag using CSS.

The changes effectively replace the StudioBetaTag component with a simpler CSS-based solution. The implementation:

  1. Removes the dedicated component import
  2. Passes the beta status through props
  3. Renders a styled span element conditionally

Also applies to: 39-39, 49-49

frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.module.css (1)

23-28: LGTM! Well-structured CSS using design system variables.

The CSS implementation:

  • Uses semantic color variables for maintainability
  • Applies consistent spacing through design system variables
  • Sets appropriate visual hierarchy with font sizing
frontend/language/src/nb.json (1)

1168-1168: Translation key appears unrelated to beta tag implementation.

This change adds a translation for the content library menu item, which seems unrelated to the PR's objective of implementing beta tag styling. Consider moving this change to a separate PR for better traceability.

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

Attention: Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.69%. Comparing base (fd014a5) to head (cee11b6).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...ures/overview/components/Navigation/Navigation.tsx 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14478      +/-   ##
==========================================
- Coverage   95.69%   95.69%   -0.01%     
==========================================
  Files        1893     1891       -2     
  Lines       24634    24633       -1     
  Branches     2823     2823              
==========================================
- Hits        23573    23572       -1     
  Misses        800      800              
  Partials      261      261              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c685f7 and d095ba6.

📒 Files selected for processing (7)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.module.css (0 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/common.module.css (1 hunks)
💤 Files with no reviewable changes (1)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.module.css
🚧 Files skipped from review as they are similar to previous changes (3)
  • frontend/libs/studio-components/src/components/StudioPageHeader/common.module.css
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx
🧰 Additional context used
🪛 GitHub Check: Typechecking and linting
frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx

[failure] 4-4:
Imports "StudioBetaTagProps" are only used as type

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build environment and run e2e test
  • GitHub Check: Testing
  • GitHub Check: CodeQL
🔇 Additional comments (7)
frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (3)

2-8: LGTM! Well-structured type definition with accessibility in mind.

The imports are appropriate, and the type definition follows TypeScript best practices with proper optional properties.


10-10: LGTM! Good accessibility practice with default ARIA label.

Exporting the constant allows for customization while ensuring accessibility by default.


12-22: LGTM! Clean and accessible implementation.

The component is well-implemented with:

  • Proper className composition using classnames utility
  • Semantic HTML using span for inline elements
  • Accessibility support with aria-label
frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx (2)

6-6: LGTM! Good practice importing the defaultAriaDescription constant.

The import statement correctly includes the defaultAriaDescription constant, making it available for testing and maintaining a single source of truth.


27-47: LGTM! Comprehensive test coverage for beta tag functionality.

The test cases effectively cover:

  • Visual styling (betaContainer class)
  • Accessibility (default and custom aria-description)
  • Edge cases (custom aria-description override)
frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx (2)

13-27: LGTM! Well-structured test cases for beta tag functionality.

The test cases provide good coverage of:

  • Visual styling (isBeta class)
  • Accessibility (default and custom aria labels)
  • Component behavior

34-34: LGTM! Good type safety improvement.

Adding the explicit RenderResult return type improves type safety and code maintainability.

@github-actions github-actions bot added the area/overview Area: Related to the overview page label Jan 22, 2025
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: 4

🔭 Outside diff range comments (1)
frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx (1)

Line range hint 54-65: Remove redundant Beta text concatenation.

The getLinkName helper still appends "Beta" text while we're moving to a CSS-based approach. This seems inconsistent with the new implementation and might lead to duplicate "Beta" indicators.

Apply this diff to fix the inconsistency:

const getLinkName = (linkItem: HeaderMenuItem): string => {
-  let name = textMock(linkItem.key);
-  if (linkItem.isBeta) {
-    name = `${name} Beta`;
-  }
-  return name;
+  return textMock(linkItem.key);
};

Also applies to: 69-74

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d095ba6 and 181afe9.

📒 Files selected for processing (16)
  • frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx (2 hunks)
  • frontend/app-development/features/overview/components/Navigation/Navigation.tsx (2 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.test.tsx (1 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (1 hunks)
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.test.tsx (4 hunks)
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.tsx (2 hunks)
  • frontend/app-development/utils/headerMenu/headerMenuUtils.ts (1 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/index.ts (0 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/index.ts (0 hunks)
  • frontend/libs/studio-components/src/index.ts (1 hunks)
  • frontend/libs/studio-components/src/style/StudioBetaTag.module.css (1 hunks)
💤 Files with no reviewable changes (5)
  • frontend/libs/studio-components/src/components/StudioBetaTag/index.ts
  • frontend/libs/studio-components/src/components/index.ts
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx
✅ Files skipped from review due to trivial changes (1)
  • frontend/libs/studio-components/src/style/StudioBetaTag.module.css
🚧 Files skipped from review as they are similar to previous changes (2)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/app-development/features/overview/components/Navigation/Navigation.tsx

[error] 9-10: This is an unexpected use of the debugger statement.

Unsafe fix: Remove debugger statement

(lint/suspicious/noDebugger)

🪛 GitHub Check: CodeQL
frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx

[notice] 6-6: Unused variable, import, function or class
Unused import defaultAriaDescription.

🪛 GitHub Check: Typechecking and linting
frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx

[failure] 6-6:
Module '"./StudioPageHeaderHeaderLink"' has no exported member 'defaultAriaDescription'.


[failure] 6-6:
'defaultAriaDescription' is declared but its value is never read.

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Build environment and run e2e test
  • GitHub Check: Testing
🔇 Additional comments (8)
frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.test.tsx (2)

52-61: Well-structured test for beta functionality!

The new test case effectively verifies the CSS-based approach for beta tags, following good testing practices and maintaining consistency with the existing test patterns.


35-35: LGTM! Consistent updates to menu item name queries.

The test cases have been consistently updated to use the plain menu item name without the "Beta" suffix, aligning with the new CSS-based approach for beta tags.

Also applies to: 47-47, 58-58, 68-68, 90-90

frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx (1)

27-35: LGTM! Well-structured test cases for beta functionality.

The new test cases effectively verify the CSS-based beta tag implementation, covering both default behavior and explicit beta state. The tests are clear, focused, and follow testing best practices.

frontend/app-development/features/overview/components/Navigation/Navigation.tsx (2)

7-8: LGTM! Clean import additions for CSS-based beta tag implementation.

The new imports align well with the PR's objective of implementing a CSS-based approach for beta tags.


24-28: LGTM! Clean implementation of CSS-based beta tag.

The conditional class name implementation using classnames is a clean and maintainable approach. This aligns well with the PR objective and follows React best practices.

frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.tsx (1)

8-8: LGTM! Clean implementation of CSS-based beta tag.

The changes effectively replace the component-based approach with a CSS-based solution, maintaining the same functionality while reducing component overhead.

Also applies to: 38-38

frontend/app-development/utils/headerMenu/headerMenuUtils.ts (2)

24-24: Verify if Create tool should be marked as beta.

The Create tool is being marked as beta. Please confirm if this is intentional and aligns with the product roadmap.


Line range hint 127-134: LGTM! Clean implementation of beta status propagation.

The mapHeaderMenuGroupToNavigationMenu function correctly forwards the isBeta property to the navigation menu items.

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: 0

♻️ Duplicate comments (1)
frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.test.tsx (1)

37-40: 🛠️ Refactor suggestion

Add accessibility testing for beta items.

While the test verifies the visual state through the isBeta class, it should also verify proper accessibility attributes.

Add assertions to verify:

 it('should set "isBeta" className for menu item that is beta', () => {
   renderLargeNavigationMenu();
   const menuItem = screen.getByRole('link', { name: menuItems[0].name });
   expect(menuItem).toHaveClass('isBeta');
+  expect(menuItem).toHaveAttribute('aria-description', expect.any(String));
 });
🧹 Nitpick comments (1)
frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.test.tsx (1)

43-46: Add negative case for accessibility attributes.

For completeness, verify that non-beta items don't have the accessibility attributes.

 it('should not set "isBeta" className for menu item that is not beta', () => {
   renderLargeNavigationMenu();
   const menuItem = screen.getByRole('link', { name: menuItems[1].name });
   expect(menuItem).not.toHaveClass('isBeta');
+  expect(menuItem).not.toHaveAttribute('aria-description');
 });
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 181afe9 and 0369c38.

📒 Files selected for processing (15)
  • frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx (2 hunks)
  • frontend/app-development/features/overview/components/Navigation/Navigation.tsx (2 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.test.tsx (2 hunks)
  • frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (2 hunks)
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.test.tsx (4 hunks)
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.tsx (2 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx (0 hunks)
  • frontend/libs/studio-components/src/components/StudioBetaTag/index.ts (0 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx (1 hunks)
  • frontend/libs/studio-components/src/components/index.ts (0 hunks)
  • frontend/libs/studio-components/src/index.ts (1 hunks)
  • frontend/libs/studio-components/src/style/StudioBetaTag.module.css (1 hunks)
💤 Files with no reviewable changes (5)
  • frontend/libs/studio-components/src/components/index.ts
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.test.tsx
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.tsx
  • frontend/libs/studio-components/src/components/StudioBetaTag/index.ts
  • frontend/libs/studio-components/src/components/StudioBetaTag/StudioBetaTag.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (8)
  • frontend/libs/studio-components/src/index.ts
  • frontend/libs/studio-components/src/style/StudioBetaTag.module.css
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.test.tsx
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.test.tsx
  • frontend/app-development/features/overview/components/Navigation/Navigation.test.tsx
  • frontend/app-development/features/overview/components/Navigation/Navigation.tsx
  • frontend/app-development/layout/PageHeader/SmallHeaderMenu/SmallHeaderMenuItem/SmallHeaderMenuItem.tsx
  • frontend/libs/studio-components/src/components/StudioPageHeader/StudioPageHeaderHeaderLink/StudioPageHeaderHeaderLink.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Build environment and run e2e test
  • GitHub Check: Testing
🔇 Additional comments (4)
frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.test.tsx (2)

24-24: Great use of role-based queries!

Using getByRole('link') instead of text-based queries follows testing best practices and ensures accessibility testing.


33-34: LGTM! Consistent use of role-based queries.

The test properly verifies both the accessibility role and the active state through CSS class.

frontend/app-development/layout/PageHeader/LargeNavigationMenu/LargeNavigationMenu.tsx (2)

5-5: LGTM! Import change aligns with the CSS-based approach.

The removal of StudioBetaTag import and reliance on StudioPageHeader is consistent with the shift to CSS-based beta tags.


39-39: Verify the CSS implementation for beta tags.

The addition of the isBeta prop looks good and aligns with the CSS-based approach. However, please ensure that:

  1. The CSS styles in HeaderLink correctly implement the beta tag appearance as shown in the PR images
  2. The styles are consistent across different viewport sizes and themes

Let's verify the CSS implementation:

✅ Verification successful

Beta tag implementation is properly implemented

The CSS implementation for beta tags is correctly implemented in frontend/libs/studio-components/src/style/StudioBetaTag.module.css with proper component integration and test coverage. The implementation follows best practices with:

  • Dedicated CSS module for beta styling
  • Type-safe props in the HeaderLink component
  • Test cases verifying the beta tag behavior
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for the CSS implementation of beta tags in HeaderLink
rg -t css "\.isBeta|beta" frontend/libs/studio-components/src/components/StudioPageHeader

Length of output: 91


Script:

#!/bin/bash
# Find HeaderLink component and its styles
echo "=== HeaderLink Component ==="
ast-grep --pattern 'export const HeaderLink = $$$'

echo -e "\n=== Beta-related styles ==="
# Search for beta-related styles in all CSS/SCSS files
rg -t css -t scss "beta|isBeta" frontend/

# Check if styles might be in styled-components
echo -e "\n=== Styled-components with beta ==="
rg "styled.*beta|isBeta.*styled" frontend/

Length of output: 3149


Script:

#!/bin/bash
# Look for HeaderLink in studio-components
echo "=== HeaderLink Component ==="
rg -t typescript "HeaderLink" frontend/libs/studio-components/src/components/StudioPageHeader --context 3

echo -e "\n=== Styled Components Beta Implementation ==="
# Search in TypeScript/TSX files for styled-components with beta
rg -t typescript "styled.*beta|isBeta.*styled|\${.*beta" frontend/libs/studio-components/src/components/StudioPageHeader

echo -e "\n=== Beta Props Interface ==="
# Look for interface/type definitions related to beta
rg -t typescript "interface.*beta|type.*beta" frontend/libs/studio-components/src/components/StudioPageHeader

Length of output: 805


Script:

#!/bin/bash
echo "=== HeaderLink Implementation ==="
# Search for HeaderLink implementation
rg "export.*HeaderLink" frontend/libs/studio-components/ -A 5

echo -e "\n=== Beta Styling Implementation ==="
# Search for beta-related styling
rg "isBeta.*=|beta.*styled|style.*beta" frontend/libs/studio-components/

echo -e "\n=== Beta Props Definition ==="
# Look for beta in types/interfaces
rg "isBeta.*:|beta.*interface|beta.*type" frontend/libs/studio-components/

Length of output: 3268

Copy link
Contributor

@wrt95 wrt95 left a comment

Choose a reason for hiding this comment

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

Very smart approach! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/overview Area: Related to the overview page frontend kind/bug Used when there is a defect / something is not working as it should. skip-documentation Issues where updating documentation is not relevant skip-releasenotes Issues that do not make sense to list in our release notes solution/studio/designer Issues related to the Altinn Studio Designer solution. team/studio-domain1
Projects
Status: 🧪 Test
Development

Successfully merging this pull request may close these issues.

Add header menu element for library in Studio
3 participants