Feature: Scroll to Line number based on Hash IE http://foo.com/p/bar#L10 will scroll to line 10. #4554
Conversation
…then on visiting scrolling back into view
|
agreed this is useful to be in core and works for me. any reason you are not using code from ace2_inner.js or scroll.js? Something like etherpad-lite/src/static/js/ace2_inner.js Line 4076 in d25010d etherpad-lite/src/static/js/scroll.js Line 258 in d25010d |
|
I can use Afaik I don't use it because sometimes using ace inner code can get a little spaghetti where just interacting with outer is clean. Will see. |
|
@webzwo0i give that a pull and test. Now it focused caret too.. But note that I'm still using new logic to handle focus because trying to use |
|
Hello, We've been using some pads for a very long time and line numbers can change over time. So being able to set an anchor would be useful too. Or, even better, being able to set an anchor in the table of contents entries. So that, https://foo.com/p/bar#EntryOneOfToc will scroll directly to EntryOneOfToc. For example, there is a table of contents on this pad https://pad.april.org/p/radio_cause_commune. It would be useful to be able to have an URL going directly to an entry of the table of contents. And, we could share this url by email for example. All the best, |
|
Try with a setTimeout, (compare with b83589f ) The default behavior with In the commit above the line that has the caret is not the first visible line in the editor, but when we now start typing or scroll to the right, no additional scrolling happens. Code that incorporates the offset of the first line and the offsetTop of the iframe is commented in the code. Can't we use the offset of the first line and the offset of the line we want to go to, to determine how much we need to scroll? IE if the offset of the first line is 30 (while scrollY is 0) and we want to go to the offset 1000, we need to scroll 1000 - 30. There is also However, there is still one big problem with my solution: When we have a lot of lines and want to go to a line in the middle of the pad, it won't work unless we increase the timeout (firefox needs a larger timeout than chromium). I can't put much time into better understanding the ACE code and don't understand what exactly has not finished loading. Maybe the method needs to be triggered in another handler. One other thing is, do we need to take into account lines with line attributes? Because the caret (first char in a line) is at I'm okay with the way you solved it, so don't mind my code, unless you have a good idea if there is another place to call the method from |
|
So ..
Ack, will do. |
|
Tested pad with 5K lines and it works fine. Tested content with varying line height and it works fine. Tested in Chrome and FF. |
|
@webzwo0i can you check my test coverage please? |
| helper.waitFor(() => { | ||
| const topOffset = parseInt(chrome$('iframe').first('iframe') | ||
| .contents().find('#outerdocbody').css('top')); | ||
| return (!topOffset); // no css top should be set. |
There was a problem hiding this comment.
see above, also applies to here
|
RE test: I added a comment there. Also, if you happen to know how to get the visible lines from the viewport, then that's probably more accurate? |
|
It's something I'm gonna have to bake in. |
|
Okay this is good for final review and merge. |
|
SonarCloud Quality Gate failed.
|
will scroll to line 10. (#4554) Includes test coverage Co-authored-by: webzwo0i <webzwo0i@c3d2.de>
will scroll to line 10. (#4554) Includes test coverage Co-authored-by: webzwo0i <webzwo0i@c3d2.de>
How to use (tm)
Clicking on side div line number to set line number hash and then on visiting scrolling back into view.
Future TODOs:
#foowill visit<h1>foo</h1>?