Skip to content
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

issue/4599-Add cursor position information on the bottom of the editor area #5379

Merged
merged 4 commits into from
Dec 6, 2024

Conversation

STF-Zero
Copy link
Contributor

@STF-Zero STF-Zero commented Dec 3, 2024

End-user friendly description of the problem this fixes or functionality that this introduces

  • Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

Give a summary of what the PR does, explaining any non-trivial design decisions

This PR add information of where the cursor is inside the editor area. The information goes like:

Row: 4, Column: 15

Here's the implementation:
image

I've made this change by only modifying

frontend/src/routes/_oh.app._index/route.tsx

  1. Added a variable cursorPosition to record the position of the cursor.
    image

  2. Dynamically change cursorPosition in handleEditorDidMount and pass it as a parameter to the sub-component code-editor-component.
    image

frontend/src/components/features/editor/code-editor-component.tsx

  1. Add the cursorPosition member variable to the component definition.
    image

  2. Add a div element to the return statement to display the cursorPosition information.
    image


Link of any specific issues this addresses
source issue issue#4599

@STF-Zero
Copy link
Contributor Author

STF-Zero commented Dec 3, 2024

This PR is renewable version of the previous PR 4626 which had been admitted by maintainers while there was some conflicts when some new changes were added into the project.
I didn't work on those conflicts but made a new fork of the up-to-date project to devote my change.
Hope this message can help you!

@enyst enyst requested a review from amanape December 3, 2024 17:49
@mamoodi
Copy link
Collaborator

mamoodi commented Dec 5, 2024

@amanape has this on his list to take a look.

Copy link
Member

@amanape amanape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you

@amanape
Copy link
Member

amanape commented Dec 6, 2024

Just need to take care of those linting errors and we're all set to merge

@STF-Zero
Copy link
Contributor Author

STF-Zero commented Dec 6, 2024

I've solved linting errors by adapting my code to a prettier style ~

@amanape amanape merged commit 2df4267 into All-Hands-AI:main Dec 6, 2024
13 checks passed
@mamoodi
Copy link
Collaborator

mamoodi commented Dec 6, 2024

@STF-Zero minor issue with this PR. The display actually overlays on top of the file.
#5438

@mamoodi
Copy link
Collaborator

mamoodi commented Dec 6, 2024

I may revert this PR because the display makes the file a little hard to see. And I want to get a release out.

@enyst
Copy link
Collaborator

enyst commented Dec 6, 2024

Sorry @STF-Zero we had to revert this, because of this #5438

Happy to take it back in when it's fixed!

@STF-Zero
Copy link
Contributor Author

STF-Zero commented Dec 7, 2024

All right, I'll try to fix it soon~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants