-
Notifications
You must be signed in to change notification settings - Fork 61
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(j-s): Don't show UI updates underneath a modal #15922
Conversation
WalkthroughThe changes in this pull request modify the logic for determining the state of indictments and user permissions in the Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
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: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (3 hunks)
Additional context used
Path-based instructions (1)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Learnings (2)
Common learnings
Learnt from: unakb PR: island-is/island.is#15378 File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100 Timestamp: 2024-06-27T14:37:07.823Z Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
Learnt from: gudjong PR: island-is/island.is#15421 File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx:55-61 Timestamp: 2024-07-03T15:43:13.884Z Learning: The `updateCase` method in the `apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx` file has its own error handling, and additional error handling in the `initialize` function is not necessary.
Learnt from: oddsson PR: island-is/island.is#14673 File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79 Timestamp: 2024-05-06T23:16:04.892Z Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (3)
Learnt from: unakb PR: island-is/island.is#15378 File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100 Timestamp: 2024-06-27T14:37:07.823Z Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
Learnt from: gudjong PR: island-is/island.is#15421 File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx:55-61 Timestamp: 2024-07-03T15:43:13.884Z Learning: The `updateCase` method in the `apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx` file has its own error handling, and additional error handling in the `initialize` function is not necessary.
Learnt from: oddsson PR: island-is/island.is#14673 File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79 Timestamp: 2024-05-06T23:16:04.892Z Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
Additional comments not posted (3)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (3)
64-65
: Approved: Updated condition forisIndictmentNew
.The modification to include
modal !== 'noModal'
in the condition forisIndictmentNew
aligns well with the PR's objective to enhance user experience by managing UI updates more effectively when modals are active.
74-76
: Approved: Refined condition foruserCanSendIndictmentToCourt
.The addition of
modal === 'noModal'
to the condition ensures that the action to send an indictment to court is only available when no modal is active, which is a prudent enhancement to prevent user errors and confusion during critical actions.
218-218
: Approved: SimplifiedmarginBottom
logic.The new logic for determining the
marginBottom
property, which now depends on whether documents can be added or an indictment can be sent to court, simplifies the previous more complex conditions. This change improves readability and maintainability of the layout configuration.
Datadog ReportBranch report: ✅ 0 Failed, 338 Passed, 0 Skipped, 1m 7.04s Total Time |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15922 +/- ##
==========================================
- Coverage 36.86% 36.79% -0.07%
==========================================
Files 6715 6693 -22
Lines 137631 136842 -789
Branches 39132 38910 -222
==========================================
- Hits 50732 50356 -376
+ Misses 86899 86486 -413
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 125 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Don't show UI updates underneath a modal
Asana
What
When a case is set to WAITING_FOR_CONFIRMATION state, there are a number of things that change on the confirmation page for prosecutors. For users who can confirm indictments, we show the fields to do so, and for others, we change the next button text to from "Send" to "Resend". Today, this happens immediately when the state change occurs, which can be confusing. This PR changes that, so that no UI update is shown until you open the case again.
Screenshots / Gifs
Pay attention to what happens underneath the modal
BEFORE
Screen.Recording.2024-09-09.at.13.58.28.mov
AFTER
Screen.Recording.2024-09-09.at.13.48.28.mov
Checklist:
Summary by CodeRabbit
New Features
Improvements
Box
component, improving readability and reducing complexity.