Remove Feature flag from #1312 #1346
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
WalkthroughThe updates introduce a close button to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TaskUpdateModal
participant CloseButton
User->>TaskUpdateModal: Opens modal
TaskUpdateModal->>User: Displays modal content with close button
User->>CloseButton: Clicks close button
CloseButton->>TaskUpdateModal: Triggers onClick handler
TaskUpdateModal->>User: Closes modal
Poem
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:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
__tests__/Unit/Components/Tasks/TaskUpdateModal.test.tsx(8 hunks)src/components/taskDetails/TaskUpdateModal.tsx(2 hunks)src/components/taskDetails/task-details.module.scss(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
__tests__/Unit/Components/Tasks/TaskUpdateModal.test.tsx
[error] 16-20: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
src/components/taskDetails/TaskUpdateModal.tsx
[error] 31-36: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (5)
src/components/taskDetails/task-details.module.scss (2)
245-251: LGTM! Good positioning setup for the modal.Adding
position: relativeto the.taskUpdateModalclass provides the necessary context for absolute positioning of the close button that was added. This is the correct approach.
253-265: Well-implemented close button styling.The styling for the close button is properly implemented with absolute positioning and appropriate hover effects. The color change from gray to black on hover provides good visual feedback to users.
src/components/taskDetails/TaskUpdateModal.tsx (1)
8-8: LGTM! Good icon choice for modal closure.The import of
IoMdClosefrom react-icons is appropriate for implementing a close button.__tests__/Unit/Components/Tasks/TaskUpdateModal.test.tsx (2)
2-2: LGTM! Proper test utility import.Good refactoring to use the specialized
renderWithRouterinstead of the basicrenderfunction, which ensures the component is properly tested within a router context.Also applies to: 10-10
71-75: LGTM! Consistent test rendering.All test cases have been properly updated to use
renderWithRouterinstead ofrender, maintaining consistency throughout the test suite.Also applies to: 88-92, 101-105, 117-121, 133-137, 143-147, 170-177
| <button | ||
| className={styles.closeButton} | ||
| onClick={() => setIsOpen(false)} | ||
| data-testid="task-update-modal-close-button" | ||
| aria-label="Close" | ||
| > | ||
| <IoMdClose size={25} /> | ||
| </button> |
There was a problem hiding this comment.
Add explicit button type for accessibility.
The close button implementation is good, but it's missing an explicit type attribute which is required for accessibility compliance.
<button
className={styles.closeButton}
onClick={() => setIsOpen(false)}
data-testid="task-update-modal-close-button"
aria-label="Close"
+ type="button"
>
<IoMdClose size={25} />
</button>📝 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.
| <button | |
| className={styles.closeButton} | |
| onClick={() => setIsOpen(false)} | |
| data-testid="task-update-modal-close-button" | |
| aria-label="Close" | |
| > | |
| <IoMdClose size={25} /> | |
| </button> | |
| <button | |
| className={styles.closeButton} | |
| onClick={() => setIsOpen(false)} | |
| data-testid="task-update-modal-close-button" | |
| aria-label="Close" | |
| type="button" | |
| > | |
| <IoMdClose size={25} /> | |
| </button> |
🧰 Tools
🪛 Biome (1.9.4)
[error] 31-36: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
| jest.fn(({ isOpen, toggle, children }) => | ||
| isOpen ? ( | ||
| <div data-testid="mock-modal"> | ||
| <button | ||
| data-testid="close-modal" | ||
| onClick={toggle} | ||
| aria-label="Close" | ||
| > | ||
| Close | ||
| </button> | ||
| {children} | ||
| </div> | ||
| ) : null | ||
| ) |
There was a problem hiding this comment.
Add button type in the Modal mock.
The Modal mock needs the same accessibility fix as the main component - an explicit button type.
<button
data-testid="close-modal"
onClick={toggle}
aria-label="Close"
+ type="button"
>
Close
</button>📝 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.
| jest.fn(({ isOpen, toggle, children }) => | |
| isOpen ? ( | |
| <div data-testid="mock-modal"> | |
| <button | |
| data-testid="close-modal" | |
| onClick={toggle} | |
| aria-label="Close" | |
| > | |
| Close | |
| </button> | |
| {children} | |
| </div> | |
| ) : null | |
| ) | |
| jest.fn(({ isOpen, toggle, children }) => | |
| isOpen ? ( | |
| <div data-testid="mock-modal"> | |
| <button | |
| data-testid="close-modal" | |
| onClick={toggle} | |
| aria-label="Close" | |
| type="button" | |
| > | |
| Close | |
| </button> | |
| {children} | |
| </div> | |
| ) : null | |
| ) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 16-20: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
ZendeAditya
force-pushed
the
remove/feature-flag
branch
from
eef33c3 to
d7433ce
Compare
|
@ZendeAditya is attempting to deploy a commit to the RDS-Team Team on Vercel. A member of the Team first needs to authorize it. |
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
why do we have removed all the test cases?
I accidentally remove first test case will add again |
|
Hey @AnujChhikara, I added the test case and remainings are running ok |
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
Can we please write better PR title?
AnujChhikara
left a comment
AnujChhikara
left a comment
There was a problem hiding this comment.
couldn’t find any proof or demo showing the changes working
|
Video added |
|
Hey @AnujChhikara can you please check now |
2 participants
Date: 18/04/2025
Developer Name: aditya-zende-1
Issue Ticket Number
#1312
Description
To Remove a feature flag from PR so that cross button will show on modal.
Remove a feature flag from #1312 PR.
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
rds.status.mp4
Test Coverage
Screenshot 1
Additional Notes