-
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
fix: remove error div from studio search #14414
Conversation
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-storybook". (The package "eslint-plugin-storybook" was not found when loaded as a Node module from the directory "/frontend/libs/studio-components".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following:
The plugin "eslint-plugin-storybook" was referenced from the config file in "frontend/libs/studio-components/.eslintrc.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. 📝 WalkthroughWalkthroughThe pull request introduces changes to several frontend components and their associated CSS modules. The modifications primarily involve styling adjustments for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
backend/src/Designer/Controllers/AppDevelopmentController.cs
(1 hunks)backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs
(1 hunks)backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs
(3 hunks)frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.module.css
(1 hunks)frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.tsx
(2 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.module.css
(0 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.tsx
(1 hunks)frontend/packages/text-editor/src/TextEditor.module.css
(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.module.css
✅ Files skipped from review due to trivial changes (2)
- frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.module.css
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CodeListsActionsBar.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Run dotnet build and test (macos-latest)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Run dotnet build and test (windows-latest)
- GitHub Check: Analyze
- GitHub Check: Run dotnet build and test (ubuntu-latest)
- GitHub Check: Run integration tests against actual gitea and db
- GitHub Check: Testing
🔇 Additional comments (5)
backend/tests/Designer.Tests/Controllers/OptionsController/GetOptionListsReferencesTests.cs (1)
92-111
: LGTM! Well-structured test for edge case.The test properly verifies the API's behavior when no options are present in the application. It covers the setup, execution, and assertion phases clearly.
backend/src/Designer/Controllers/AppDevelopmentController.cs (1)
550-557
: LGTM! Improved error handling with proper HTTP status codes.The changes align with RESTful practices by:
- Removing unnecessary exception handling.
- Using 204 No Content when no options are found.
backend/src/Designer/Infrastructure/GitRepository/AltinnAppGitRepository.cs (1)
801-801
: LGTM! Verify C# version compatibility.The change improves error handling by returning an empty array instead of throwing an exception. The use of collection expression syntax
[]
is clean but requires C# 12.Run the following script to verify the C# version used in the project:
✅ Verification successful
Collection expression syntax is compatible with project configuration ✅
The codebase targets .NET 9.0 which includes C# 12 by default, making the collection expression syntax
[]
fully supported.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check C# language version in project files # Test: Search for LangVersion in csproj files. Expect: preview or 12.0 or higher rg -l 'LangVersion' --glob '*.csproj'Length of output: 82
Script:
#!/bin/bash # Find the project file containing AltinnAppGitRepository.cs fd AltinnAppGitRepository.cs --exec dirname {} | xargs -I {} find {} -maxdepth 1 -name "*.csproj" # Check content of all relevant project files and global props echo "=== Project files content ===" fd . -e csproj -e props --exec cat {}Length of output: 33978
frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.tsx (1)
4-4
: Verify the CSS implementation for error div removal.The CSS module import has been added, but the actual CSS implementation that removes the error div is not visible in the provided files. Please ensure that the CSS changes don't affect other critical styling aspects of the component.
✅ Verification successful
CSS implementation for error div removal is correct ✓
The CSS module correctly hides error elements using a scoped selector, which is a clean and safe implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the StudioSearch.module.css file and its contents fd "StudioSearch.module.css" | xargs catLength of output: 96
frontend/packages/text-editor/src/TextEditor.module.css (1)
21-21
: Verify the necessity of alignment changes.The alignment changes from
center
toflex-end
in.textEditorTopRow
and.filterAndSearch
classes appear unrelated to the PR's objective of removing the error div from StudioSearch. Please clarify if these changes should be in a separate PR focused on layout improvements.Also applies to: 28-28
✅ Verification successful
Alignment changes are consistent with codebase patterns
The changes from
center
toflex-end
align with the established UI patterns across the codebase, where similar interactive components and action bars consistently useflex-end
alignment. These changes appear to be valid improvements for UI consistency.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any related alignment changes or discussions rg "align-items.*flex-end" --type cssLength of output: 1081
frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.tsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14414 +/- ##
=======================================
Coverage 95.66% 95.66%
=======================================
Files 1880 1880
Lines 24447 24448 +1
Branches 2810 2810
=======================================
+ Hits 23387 23388 +1
Misses 800 800
Partials 260 260 ☔ View full report in Codecov by Sentry. |
frontend/libs/studio-components/src/components/StudioSearch/StudioSearch.module.css
Outdated
Show resolved
Hide resolved
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.
Testet OK 😊
Description
When adding
StudioSearch
component, it did not fit in with the content library.Prod:

This PR:

Add css in
StudioSearch
to visually remove a div that will be in the DOM, and occupy some height, also when the component has no error.OBS: this will also hide the error if the component would ever return an error that we want to display. None of our usages of the
StudioSearch
are performing any error-handling per today, but maybe an issue in the future if we need the handle an error before we update to the next version of DS where this issue is fixed.Related Issue(s)
Verification
Summary by CodeRabbit
Style
.searchField
class from CodeListsActionsBar.Refactor