-
Notifications
You must be signed in to change notification settings - Fork 76
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
feat: add library as beta to studio page header with news #14491
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14491 +/- ##
=======================================
Coverage 95.71% 95.71%
=======================================
Files 1904 1904
Lines 24804 24805 +1
Branches 2840 2840
=======================================
+ Hits 23741 23743 +2
Misses 802 802
+ Partials 261 260 -1 ☔ View full report in Codecov by Sentry. |
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.
Code changes look good! 💪
I guess merging here is blocked by the text-resource issue for code lists?
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.tsx (1)
3-3
: LGTM! Consider adding type safety for classnames.The implementation correctly adds the beta tag styling to the library header and maintains proper internationalization.
Consider using a typed version of classnames for better type safety:
-import cn from 'classnames'; +import cn from 'classnames/bind'; +const styles = cn.bind(classes); -<StudioHeading size='small' className={cn(classes.headingText, studioBetaTagClasses.isBeta)}> +<StudioHeading size='small' className={styles('headingText', studioBetaTagClasses.isBeta)}>Also applies to: 6-6, 14-16
frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.test.tsx (1)
20-26
: Enhance test coverage for class assertions.The test correctly verifies the beta tag, but could be more comprehensive.
Consider enhancing the test to verify both classes and use a more specific selector:
it('renders the content library header with isBeta class', () => { renderLibraryHeader(); const libraryHeader = screen.getByRole('heading', { name: textMock('app_content_library.library_heading'), }); - expect(libraryHeader).toHaveClass('isBeta'); + expect(libraryHeader).toHaveClass('headingText', 'isBeta'); + // Or for more specific testing: + expect(libraryHeader.className).toMatch(/headingText.*isBeta/); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
frontend/app-development/enums/HeaderMenuItemKey.ts
(1 hunks)frontend/app-development/features/overview/components/News/NewsContent/news.nb.json
(1 hunks)frontend/app-development/utils/headerMenu/headerMenuUtils.test.ts
(1 hunks)frontend/app-development/utils/headerMenu/headerMenuUtils.ts
(2 hunks)frontend/language/src/nb.json
(2 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/ImagesPage/ImagesPage.test.tsx
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/ImagesPage/ImagesPage.tsx
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.module.css
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.test.tsx
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryHeader/LibraryHeader.module.css
⏰ 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-content-library/src/ContentLibrary/LibraryBody/pages/ImagesPage/ImagesPage.tsx (1)
27-27
: LGTM! Good user experience improvement.The change from "no content" to "coming soon" message provides better feedback to users about the upcoming feature.
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/ImagesPage/ImagesPage.test.tsx (1)
25-26
: LGTM! Test updated correctly.The test has been properly updated to match the new "coming soon" message, maintaining test coverage for the component change.
frontend/language/src/nb.json (1)
47-47
: LGTM! Translation added correctly.The new translation key
app_content_library.images.coming_soon
with the Norwegian text "Snart blir det mulig å laste opp bilder i biblioteket. Vi jobber med saken!" is well-structured and provides clear information to users.frontend/app-development/enums/HeaderMenuItemKey.ts (1)
8-8
: LGTM!The new enum entry follows the established naming conventions and is correctly placed before the
None
entry.frontend/app-development/utils/headerMenu/headerMenuUtils.ts (2)
6-13
: LGTM!The BookIcon import is appropriately added and grouped with other icon imports.
60-67
: LGTM!The ContentLibrary menu item is well-structured with:
- Appropriate icon choice
- Correct repository type restriction
- Logical grouping under Tools
- Clear beta status indication
frontend/app-development/utils/headerMenu/headerMenuUtils.test.ts (1)
174-180
: LGTM!The test expectations are correctly updated to include the new ContentLibrary menu item while maintaining the existing order.
{ | ||
"date": "2025-01-27", | ||
"title": "Biblioteket er nå tilgjengelig fra menylinjen", | ||
"content": "Vi har lagd et nytt bibliotek! Her finner du nå kodelistene dine. Vi jobber med å utvide biblioteket til at du kan finne eller lagre andre ting der, som bilder og tekster." | ||
}, |
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.
Update the news entry date to the current month.
The news entry date is set to "2025-01-27" which is in the future. Since the current date is January 2025, please update it to a date within the current month.
Apply this diff to fix the date:
- "date": "2025-01-27",
+ "date": "2025-01-15",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
{ | |
"date": "2025-01-27", | |
"title": "Biblioteket er nå tilgjengelig fra menylinjen", | |
"content": "Vi har lagd et nytt bibliotek! Her finner du nå kodelistene dine. Vi jobber med å utvide biblioteket til at du kan finne eller lagre andre ting der, som bilder og tekster." | |
}, | |
{ | |
"date": "2025-01-15", | |
"title": "Biblioteket er nå tilgjengelig fra menylinjen", | |
"content": "Vi har lagd et nytt bibliotek! Her finner du nå kodelistene dine. Vi jobber med å utvide biblioteket til at du kan finne eller lagre andre ting der, som bilder og tekster." | |
}, |
Description
Add library as beta to studio page header with news:
Related Issue(s)
Verification
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization
User Interface