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

[CLOSED] Fix find first result centering #4018

Open
core-ai-bot opened this issue Aug 29, 2021 · 7 comments
Open

[CLOSED] Fix find first result centering #4018

core-ai-bot opened this issue Aug 29, 2021 · 7 comments

Comments

@core-ai-bot
Copy link
Member

Issue by JeffryBooher
Tuesday Jun 25, 2013 at 20:46 GMT
Originally opened as adobe/brackets#4339



JeffryBooher included the following code: https://github.com/adobe/brackets/pull/4339/commits

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Tuesday Jun 25, 2013 at 20:47 GMT


@dangoor this should be good to go.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Tuesday Jun 25, 2013 at 22:25 GMT


line 1 out of viewport-1
line 41 in viewport

See screenshots. I disabled the shell menus by commenting lines 272-274 in brackets.js to move the editorholder position downwards.

  • Screenshot 1: Line 1 is above the editor viewport but is reported as visible
  • Screenshot 2: Line 41 is visible in the viewport but is **not* reported visible

@core-ai-bot
Copy link
Member Author

Comment by dangoor
Wednesday Jun 26, 2013 at 01:21 GMT


It looks like we'll want to take this in sprint 28 after the bug Jason noticed has been squashed.

(What Jeff said is true: this shouldn't affect the find feature. the trouble I see is that isLineVisible is new public API that should work for other cases.)

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Jun 26, 2013 at 19:45 GMT


@jasonsanjose@dangoor try it now. I'm using scroll position and page relative result checking which so far appears to be bullet-proof

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Jun 26, 2013 at 19:46 GMT


scratch that... "Local" coordinates not "Page"... I had tried "Page" earlier without success. "Local" is what we want...

@core-ai-bot
Copy link
Member Author

Comment by JeffryBooher
Wednesday Jul 10, 2013 at 18:47 GMT


@jasonsanjose@dangoor I may have dropped the ball on this. I should have let you all know that it is ready to merge now.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Thursday Jul 11, 2013 at 18:36 GMT


Looks good. Merging.

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

No branches or pull requests

1 participant