Repository navigation
enhance:change-hover-color-action-button - #1978
Merged
Merged
Conversation
Contributor
Summary by CodeRabbit
WalkthroughChanged ActionButton styles: light-mode blue hex updated to #1D7BD7; removed several dark-mode Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (3)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/components/ActionButton.tsx (1)
14-15: Consider extracting styles for better maintainability.The long className string works but could be improved for readability and maintainability.
Consider refactoring the styles:
-const ActionButton: React.FC<ActionButtonProps> = ({ url, onClick, tooltipLabel, children }) => { - const baseStyles = - 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center bg-transparent text-blue-600 border-[#0D6EFD] hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-white dark:hover:bg-blue-600' +const ActionButton: React.FC<ActionButtonProps> = ({ url, onClick, tooltipLabel, children }) => { + const baseStyles = [ + 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center', + 'bg-transparent text-blue-600 border-[#0D6EFD]', + 'hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white', + 'dark:border-sky-600 dark:text-white dark:hover:bg-blue-600' + ].join(' ')frontend/__tests__/unit/components/ActionButton.test.tsx (1)
120-125: Consider enhancing the test for more comprehensive coverage.The current test only verifies class presence. Consider testing both the old classes are removed and the new ones are present.
- it(`changes background and text color on hover`, () => { - render(<ActionButton>Test Button</ActionButton>); - const button = screen.getByRole('button') - expect(button).toHaveClass('dark:hover:bg-blue-600') - expect(button).toHaveClass('dark:text-white') - }) + it('applies correct dark mode styling classes', () => { + render(<ActionButton>Test Button</ActionButton>) + const button = screen.getByRole('button') + + // Verify new dark mode classes are present + expect(button).toHaveClass('dark:hover:bg-blue-600') + expect(button).toHaveClass('dark:text-white') + + // Verify old classes are not present + expect(button).not.toHaveClass('dark:hover:bg-sky-100') + expect(button).not.toHaveClass('dark:text-sky-600') + })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/__tests__/unit/components/ActionButton.test.tsx(1 hunks)frontend/src/components/ActionButton.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: when testing react page components that use mocked form components, validation logic should be teste...
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/components/ActionButton.test.tsx
📚 Learning: in react applications, button elements should always have an explicit type attribute (type="button",...
Learnt from: Rajgupta36
PR: OWASP/Nest#1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.
Applied to files:
frontend/src/components/ActionButton.tsx
🔇 Additional comments (2)
frontend/src/components/ActionButton.tsx (1)
15-15: LGTM! Dark mode styling updated correctly.The changes successfully replace
skycolors withbluecolors as specified in the objectives. The hover backgrounddark:hover:bg-blue-600and text colordark:text-whiteprovide good contrast and align with the design guidelines.frontend/__tests__/unit/components/ActionButton.test.tsx (1)
120-125: LGTM! Test verifies the updated dark mode classes.The test correctly checks for the presence of the new dark mode classes
dark:hover:bg-blue-600anddark:text-whitethat were updated in the component.
Comment on lines
+120
to
+125
| it(`changes background and text color on hover`, () => { | ||
| render(<ActionButton>Test Button</ActionButton>); | ||
| const button = screen.getByRole('button') | ||
| expect(button).toHaveClass('dark:hover:bg-blue-600') | ||
| expect(button).toHaveClass('dark:text-white') | ||
| }) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix formatting inconsistencies and enhance test coverage.
The test correctly verifies the presence of the new CSS classes, but has some formatting issues and could be more comprehensive.
Apply this diff to fix formatting and improve the test:
- it(`changes background and text color on hover`, () => {
- render(<ActionButton>Test Button</ActionButton>);
+ it('applies dark mode hover styles', () => {
+ render(<ActionButton>Test Button</ActionButton>)
const button = screen.getByRole('button')
- expect(button).toHaveClass('dark:hover:bg-blue-600')
- expect(button).toHaveClass('dark:text-white')
+ expect(button).toHaveClass('dark:hover:bg-blue-600')
+ expect(button).toHaveClass('dark:text-white')
})Changes made:
- Use consistent single quotes for test description
- Remove unnecessary semicolon after render call
- Remove trailing spaces
- Improve test name to be more descriptive
📝 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.
Suggested change
| it(`changes background and text color on hover`, () => { | |
| render(<ActionButton>Test Button</ActionButton>); | |
| const button = screen.getByRole('button') | |
| expect(button).toHaveClass('dark:hover:bg-blue-600') | |
| expect(button).toHaveClass('dark:text-white') | |
| }) | |
| it('applies dark mode hover styles', () => { | |
| render(<ActionButton>Test Button</ActionButton>) | |
| const button = screen.getByRole('button') | |
| expect(button).toHaveClass('dark:hover:bg-blue-600') | |
| expect(button).toHaveClass('dark:text-white') | |
| }) |
🤖 Prompt for AI Agents
In frontend/__tests__/unit/components/ActionButton.test.tsx around lines 120 to
125, fix formatting by using consistent single quotes for the test description,
removing the unnecessary semicolon after the render call, and eliminating
trailing spaces. Also, improve the test name to be more descriptive about what
it verifies, ensuring the test clearly states it checks the background and text
color changes on hover.
| const ActionButton: React.FC<ActionButtonProps> = ({ url, onClick, tooltipLabel, children }) => { | ||
| const baseStyles = | ||
| 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center bg-transparent text-blue-600 border-[#0D6EFD] hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-sky-600 dark:hover:bg-sky-100' | ||
| 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center bg-transparent text-blue-600 border-[#0D6EFD] hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-white dark:hover:bg-blue-600' |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Approve the dark mode color changes but suggest refactoring for maintainability.
The color changes from sky to blue align perfectly with the PR objectives. However, the extremely long class string has maintainability issues and contains duplicate color declarations.
Apply this diff to improve readability and remove conflicts:
- const baseStyles =
- 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center bg-transparent text-blue-600 border-[#0D6EFD] hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-white dark:hover:bg-blue-600'
+ const baseStyles = [
+ 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center',
+ 'bg-transparent border-[#0D6EFD] hover:bg-[#0D6EFD]',
+ 'text-[#0D6EFD] hover:text-white',
+ 'dark:border-sky-600 dark:text-white dark:hover:bg-blue-600'
+ ].join(' ')This removes the duplicate text-blue-600 declaration and improves readability by grouping related styles.
📝 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.
Suggested change
| 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center bg-transparent text-blue-600 border-[#0D6EFD] hover:bg-[#0D6EFD] text-[#0D6EFD] hover:text-white dark:border-sky-600 dark:text-white dark:hover:bg-blue-600' | |
| const baseStyles = [ | |
| 'flex items-center gap-2 px-2 py-2 rounded-md border transition-all text-nowrap justify-center', | |
| 'bg-transparent border-[#0D6EFD] hover:bg-[#0D6EFD]', | |
| 'text-[#0D6EFD] hover:text-white', | |
| 'dark:border-sky-600 dark:text-white dark:hover:bg-blue-600' | |
| ].join(' ') |
🤖 Prompt for AI Agents
In frontend/src/components/ActionButton.tsx at line 15, the className string is
too long and contains duplicate color declarations, which affects
maintainability. Refactor the class string by removing duplicate color classes
like 'text-blue-600' and group related styles logically to improve readability
and avoid conflicts, while keeping the approved dark mode color changes intact.
Contributor
Author
kasya
requested changes
Aug 6, 2025
| it(`changes background and text color on hover`, () => { | ||
| render(<ActionButton>Test Button</ActionButton>) | ||
| const button = screen.getByRole('button') | ||
| expect(button).toHaveClass('hover:bg-[#1D7BD7]') |
Collaborator
There was a problem hiding this comment.
@VijeshVS These are not really testing the hover color. The class might be there, but another class can easily override it.
Please check InfoBlock.test.tsx to see how to mock hover effect in tests and update these.
Contributor
Author
There was a problem hiding this comment.
okay understood !! will work on this
Collaborator
|
@VijeshVS Hello! Do you plan to update this PR with requested changes? |
kasya
enabled auto-merge
August 15, 2025 23:49
|
Dhirajsharma2060
pushed a commit
to Dhirajsharma2060/Nest
that referenced
this pull request
Aug 16, 2025
* enhance:change-hover-color-action-button * Update colors and tests * Add comprehensive hover state check --------- Co-authored-by: Kate <kate@kgthreads.com>
2 tasks done
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Aug 28, 2025
* test: add unit tests for SortBy component * test: update SortBy component tests for prop naming and accessibility * test: import React in SortBy component tests for consistency * made some chnages * test: update SortBy component tests for improved accessibility and clarity * test: ensure order toggle button functionality in SortBy component tests * feat: add has_full_name filter to top contributors query and model * fix: update regex for user name filtering in RepositoryContributor queryset * Update code * test: enhance RepositoryContributor tests with full name filter validations * refactor: use class constants for regex pattern and mock data in RepositoryContributor tests * test: fix regex filter call in has_full_name filter validation * Update code * Update tests * Update code * Update tests * Update test * feat: periodic job to sync OWASP Employee badges (#1762) * fix: optimize badge assignment logic and improve test coverage for sync_user_badges command * refactor: clean up comments in sync_user_badges test cases for clarity * refactor: remove unused mock_badge fixture from sync_user_badges test * feat: add management command to sync OWASP Staff badges and corresponding tests * refactor: optimize badge assignment logic for OWASP Staff badge sync * Update code * feat: add badges field to User model and update badge sync logic for OWASP staff feat:add badges field to User model and updated badge sync logic OWASP staff * feat: add badges field to User model and update related tests for badge synchronization * test: ensure no badge operations are performed when no employees are eligible * test: enhance user filter logic and improve badge assignment messages in tests suggested by coderabbit * test: refine user filter logic for badge assignment and enhance handling of keyword and positional arguments * test: refactor user filter logic in badge assignment tests for improved clarity and functionality * test: simplify user filter logic in badge assignment tests for improved readability and maintainability * test: simplify user filter logic by extracting helper functions for improved clarity and maintainability * test: update patch paths for badge and user objects in sync_user_badges tests for consistency * Update code * test: update patch paths for badge and user objects in TestSyncUserBadgesCommand for consistency * Fix #1912: Added test for SecondaryCard component (#2069) * Fix #1912: Added test for SecondaryCard component * Added newline at end of SecondaryCard.test.tsx * Fix make check --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * test: add unit tests for GeneralCompliantComponent (#2018) * test: add unit tests for GeneralCompliantComponent * fix(#1835): Refactor tests based on reviewer feedback * Fix make check and revert unnecessary changes * Fix issues --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * Added Light Theme Support for Contribution HeatMap (#2072) * Added Light Theme Support for Contribution HeatMap * Added Light Theme Support for Contribution HeatMap (Updated) * Added Light Theme Support for Contribution HeatMap (Updated) * Update color scheme for the heatmap * Update code --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * feat: add comprehensive unit tests for DisplayIcon component (#2048) * feat: add comprehensive unit tests for DisplayIcon component - 30+ test cases covering rendering, props, edge cases, and accessibility - Mock all dependencies and ensure SonarQube compliance - Test both snake_case and camelCase property conventions * Fixed issues flagged by the bot * Fixed issues flagged by the bot * Fixed issues flagged by the bot and added unhover word in cspell/custom-dict.txt --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * enhance:change-hover-color-action-button (#1978) * enhance:change-hover-color-action-button * Update colors and tests * Add comprehensive hover state check --------- Co-authored-by: Kate <kate@kgthreads.com> * Test: add unit tests for BarChart component #1801 (#1904) * Add unit tests for BarChart component - Added comprehensive test cases for BarChart component - Tests cover rendering, data handling, and prop validation - Follows project testing standards * Add unit tests for BarChart component * Add unit tests for BarChart component * Fix linting issues * Fixed unit tests for BarChart component * Fixed unit tests for BarChart component By CodeRabbit * Fixed unit tests for BarChart component By CodeRabbit * Fix tests and make check issues * Fixed unit tests for BarChart component * Fix issues and add more tests --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * Project Dasbored Drop Down Test case has been made (#2052) * Project Dasbored Drop Down Test case has been made * Changes made * Made the changes for the code check and test cases * Changes has been made so that it works with eslint * Fix make check issues --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * fix: update command references in badge management tests * Update code * fix: update badge management command to correctly filter users by badge relationship * style: format code for better readability in badge assignment logic * fix: update badge assignment logic to use UserBadge model and remove unnecessary user management calls * fix: include owasp Makefile in backend Makefile * fix: improve badge removal logic for non-OWASP employees * fix: add unique constraint for user and badge in UserBadge model * Update code * Revert volumes * feat: add is_active field to UserBadge model and update badge assignment logic * style: format migration file for userbadge is_active field * test: mock return value for UserBadge get_or_create in badge sync tests * fix: correct docstring for nest_update_badges management command tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: rasesh <70015367+rasesh-here@users.noreply.github.com> Co-authored-by: Kate Golovanova <kate@kgthreads.com> Co-authored-by: Karmjeet Chauhan <addresskrish@gmail.com> Co-authored-by: Abhay Bahuguna <77850911+Abhay056@users.noreply.github.com> Co-authored-by: Rizwaan Ansari <108953802+RizWaaN3024@users.noreply.github.com> Co-authored-by: Vijesh Shetty <84760221+VijeshVS@users.noreply.github.com> Co-authored-by: Mohd Waqar Ahemed <98345819+MohdWaqar98@users.noreply.github.com> Co-authored-by: SOHAMPAL23 <palsoham074@gmail.com>
Dishant1804
pushed a commit
to Dishant1804/Nest
that referenced
this pull request
Sep 6, 2025
* test: add unit tests for SortBy component * test: update SortBy component tests for prop naming and accessibility * test: import React in SortBy component tests for consistency * made some chnages * test: update SortBy component tests for improved accessibility and clarity * test: ensure order toggle button functionality in SortBy component tests * feat: add has_full_name filter to top contributors query and model * fix: update regex for user name filtering in RepositoryContributor queryset * Update code * test: enhance RepositoryContributor tests with full name filter validations * refactor: use class constants for regex pattern and mock data in RepositoryContributor tests * test: fix regex filter call in has_full_name filter validation * Update code * Update tests * Update code * Update tests * Update test * feat: periodic job to sync OWASP Employee badges (OWASP#1762) * fix: optimize badge assignment logic and improve test coverage for sync_user_badges command * refactor: clean up comments in sync_user_badges test cases for clarity * refactor: remove unused mock_badge fixture from sync_user_badges test * feat: add management command to sync OWASP Staff badges and corresponding tests * refactor: optimize badge assignment logic for OWASP Staff badge sync * Update code * feat: add badges field to User model and update badge sync logic for OWASP staff feat:add badges field to User model and updated badge sync logic OWASP staff * feat: add badges field to User model and update related tests for badge synchronization * test: ensure no badge operations are performed when no employees are eligible * test: enhance user filter logic and improve badge assignment messages in tests suggested by coderabbit * test: refine user filter logic for badge assignment and enhance handling of keyword and positional arguments * test: refactor user filter logic in badge assignment tests for improved clarity and functionality * test: simplify user filter logic in badge assignment tests for improved readability and maintainability * test: simplify user filter logic by extracting helper functions for improved clarity and maintainability * test: update patch paths for badge and user objects in sync_user_badges tests for consistency * Update code * test: update patch paths for badge and user objects in TestSyncUserBadgesCommand for consistency * Fix OWASP#1912: Added test for SecondaryCard component (OWASP#2069) * Fix OWASP#1912: Added test for SecondaryCard component * Added newline at end of SecondaryCard.test.tsx * Fix make check --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * test: add unit tests for GeneralCompliantComponent (OWASP#2018) * test: add unit tests for GeneralCompliantComponent * fix(OWASP#1835): Refactor tests based on reviewer feedback * Fix make check and revert unnecessary changes * Fix issues --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * Added Light Theme Support for Contribution HeatMap (OWASP#2072) * Added Light Theme Support for Contribution HeatMap * Added Light Theme Support for Contribution HeatMap (Updated) * Added Light Theme Support for Contribution HeatMap (Updated) * Update color scheme for the heatmap * Update code --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * feat: add comprehensive unit tests for DisplayIcon component (OWASP#2048) * feat: add comprehensive unit tests for DisplayIcon component - 30+ test cases covering rendering, props, edge cases, and accessibility - Mock all dependencies and ensure SonarQube compliance - Test both snake_case and camelCase property conventions * Fixed issues flagged by the bot * Fixed issues flagged by the bot * Fixed issues flagged by the bot and added unhover word in cspell/custom-dict.txt --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * enhance:change-hover-color-action-button (OWASP#1978) * enhance:change-hover-color-action-button * Update colors and tests * Add comprehensive hover state check --------- Co-authored-by: Kate <kate@kgthreads.com> * Test: add unit tests for BarChart component OWASP#1801 (OWASP#1904) * Add unit tests for BarChart component - Added comprehensive test cases for BarChart component - Tests cover rendering, data handling, and prop validation - Follows project testing standards * Add unit tests for BarChart component * Add unit tests for BarChart component * Fix linting issues * Fixed unit tests for BarChart component * Fixed unit tests for BarChart component By CodeRabbit * Fixed unit tests for BarChart component By CodeRabbit * Fix tests and make check issues * Fixed unit tests for BarChart component * Fix issues and add more tests --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * Project Dasbored Drop Down Test case has been made (OWASP#2052) * Project Dasbored Drop Down Test case has been made * Changes made * Made the changes for the code check and test cases * Changes has been made so that it works with eslint * Fix make check issues --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com> * fix: update command references in badge management tests * Update code * fix: update badge management command to correctly filter users by badge relationship * style: format code for better readability in badge assignment logic * fix: update badge assignment logic to use UserBadge model and remove unnecessary user management calls * fix: include owasp Makefile in backend Makefile * fix: improve badge removal logic for non-OWASP employees * fix: add unique constraint for user and badge in UserBadge model * Update code * Revert volumes * feat: add is_active field to UserBadge model and update badge assignment logic * style: format migration file for userbadge is_active field * test: mock return value for UserBadge get_or_create in badge sync tests * fix: correct docstring for nest_update_badges management command tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: rasesh <70015367+rasesh-here@users.noreply.github.com> Co-authored-by: Kate Golovanova <kate@kgthreads.com> Co-authored-by: Karmjeet Chauhan <addresskrish@gmail.com> Co-authored-by: Abhay Bahuguna <77850911+Abhay056@users.noreply.github.com> Co-authored-by: Rizwaan Ansari <108953802+RizWaaN3024@users.noreply.github.com> Co-authored-by: Vijesh Shetty <84760221+VijeshVS@users.noreply.github.com> Co-authored-by: Mohd Waqar Ahemed <98345819+MohdWaqar98@users.noreply.github.com> Co-authored-by: SOHAMPAL23 <palsoham074@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed change
Resolves #1955
Checklist
make check-testlocally; all checks and tests passed.