[Bugfix:InstructorUI] Fix missing cursor in config editor#12552
[Bugfix:InstructorUI] Fix missing cursor in config editor#12552shreyashrpawar wants to merge 7 commits into
Conversation
automateprojectmangement
Bot
moved this from Seeking Reviewer
to In Review
in Submitty Development
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12552 +/- ##
=========================================
Coverage 21.66% 21.66%
Complexity 9637 9637
=========================================
Files 268 268
Lines 36222 36222
Branches 487 487
=========================================
Hits 7847 7847
Misses 27892 27892
Partials 483 483
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
shreyashrpawar
changed the title
roye2
left a comment
roye2
left a comment
There was a problem hiding this comment.
I did a code and functionality review and the changes look good. I was able to reproduce the bug in main and then see that the changes fixed the bug and the cursor displayed correctly. The code looks good as well. I am pretty sure the CI test that is failing is just flaky and not to do with your changes.
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
|
@shreyashrpawar I am not sure but I think the Cypress Gradeable test has an issue with the Pam e2e test that it does that causes it to be flaky, I've seen it show an error various other times. I can't think of a reason why your changes would cause that specific failure anyway. |
|
@roye2 Okay Thanks |
automateprojectmangement
Bot
moved this from Awaiting Maintainer Review
to In Review
in Submitty Development
github-project-automation
Bot
moved this from In Review
to Awaiting Maintainer Review
in Submitty Development
williamjallen
left a comment
williamjallen
left a comment
There was a problem hiding this comment.
@shreyashrpawar I believe the approach in #12554 is better. Can you please leave a review on #12554? Without a good reason, I plan to close this in favor of #12554.
github-project-automation
Bot
moved this from Awaiting Maintainer Review
to Work in Progress
in Submitty Development
|
@williamjallen I checked the approach from #12554 and it does not works. I wrote the comment on that PR. |
automateprojectmangement
Bot
moved this from Work in Progress
to In Review
in Submitty Development
@dagemcn Thanks for the review and for verifying the fix! And As a contributor, I don't have the permissions to re-run of the GitHub Actions. |
I have approved the tests to run again and merged main into this branch. Assuming all tests pass, you should be good to go. |
roye2
left a comment
roye2
left a comment
There was a problem hiding this comment.
I just left my review in the duplicate PR (PR #12554 ) as I found it to work correctly on my machine. Using codemirror to update the cursor is the correct approach. If you would be able to explain why the other PR didn't work, that would be great.
github-project-automation
Bot
moved this from In Review
to Work in Progress
in Submitty Development
4 participants
Why is this Change Important & Necessary?
Fixes #12551
To edit any file having the live blinking cursor is beneficial. Earlier, that cursor is not visible but, this PR will fix it.
What is the New Behavior?
Now you can view the cursor as showing in the following image:
What steps should a reviewer take to reproduce or test the bug or new feature?