-
Notifications
You must be signed in to change notification settings - Fork 406
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
Fixed #931 Fix Out-of-Bounds Issue #932
Conversation
@shivareddy6 could you review the PR and the related issue? |
Hey @SURAJ-SHARMA27, I tried out your solution, and it works! But I'm thinking, maybe we could handle the For example, you can include |
Also I would suggest you to just make changes related to the issue #931 i.e., just resolve the error part and not change UI of remove modal. |
a512c6b
to
3b0952c
Compare
@shivareddy6 , could you please review the PR and the related issue? I have made changes based on the feedback. First, I synced the project. Then, I found the issue persisted with second last step, and I fixed that by avoiding conditional rendering at every place and using the following logic: // Your code here
useEffect(() => {
// ...
}, [currentStep, stepsData]);
const isValidStep =
currentStep >= 0 && currentStep < (stepsData?.length || 0);
const validStepId = isValidStep
? stepsData && stepsData[currentStep]?.id
: null; @shivareddy6 Could you please review both the issue #1030 and the pull request #1031 ?" |
Hey @shivareddy6 , |
Hey @rohitPandey469 yes, we can address this issue at the action level by checking before dispatching the current step. However, an alternative approach is to handle it on the frontend where we receive the current step value. In my pull request #1031 , I've already resolved this issue by checking if the current step is beyond the array length. and it is working fine Here's the code snippet:
This ensures that the current step is within the valid range and prevents it from exceeding the array length. |
Description
I fixed an issue where the website would break and display a white screen when attempting to remove the last step from a tutorial that contains only two steps. I also resolved the error thrown in the browser's console.
The issue occurred when {currentStep} had a value that was out of bounds, and {stepsData} was either undefined or did not contain enough elements to support the access.
To fix this issue, I added conditional checks throughout the code to ensure that properties of stepsData and other related variables were accessed only when they existed and when {currentStep} was within a valid range.
I have improved the UI of the modal box that opens upon clicking "Remove Step." The UI is now more user-friendly, providing a better overall user experience.
Related Issue
resolved issue #931
Motivation and Context
The changes were required to address a critical bug that caused the website to break under certain conditions.
How Has This Been Tested?
I have tested it locally
Screenshots or GIF (In case of UI changes):
soln.mp4
Types of changes
Checklist: