dproofreaders icon indicating copy to clipboard operation
dproofreaders copied to clipboard

Allow image to scroll beyond edges

Open 70ray opened this issue 1 year ago • 40 comments

This allows the image to scroll to halfway across the image to left and right and 80% of the way upwards. These amounts could be changed and it could be made to work differently for vertical and horizontal layouts.

Sandbox at: https://www.pgdp.org/~rp31/c.branch/image_overscroll

70ray avatar Jan 06 '24 17:01 70ray

Initial opinion using the sandbox, my remembered preference for scroll position put the image halfway not visible. Do we need to migrate the saved local storage data?

Thanks for testing. That shouldn't happen. I'll look into it. It should be fixed now.

70ray avatar Jan 07 '24 05:01 70ray

Sandbox updated.

70ray avatar Jan 07 '24 08:01 70ray

This is very cool. It functions the way I was hoping with one bug: I can pan the image past the left, top, and right sides of the viewport but not the bottom.

cpeel avatar Jan 08 '24 05:01 cpeel

This is very cool. It functions the way I was hoping with one bug: I can pan the image past the left, top, and right sides of the viewport but not the bottom.

From reading the code, I assumed that was intentional. I would put it differently -- I can scroll past the bottom of the page, but not above the top of the page.

There's other thing that I'm not quite sure how to describe. The amount of pan room seems to be dependent on the height or width of the image pane, not the image (or maybe it has to do with the relationship between the image and the image pane). For example: if I resize the proofing window to make it shorter, but don't resize the image correspondingly, I can't pan as far in any direction. If I'm in horizontal mode and I make the image the full width of the image pane, I can't scroll past either the top or the bottom.

If I resize the image to fit the width of the image pane (in horizontal mode), I can scroll the correct percentages left and right, but can't scroll past the top or the bottom of the page. Similarly, if I am silly enough to resize the image to fit the height of the pane in horizontal mode, the top and bottom match the coded percentages pretty well, but left and right are pretty far off from the correct percentages (although I think the point is to scroll the image halfway off the screen. It does slightly more than halfway off the screen, but close enough).

Let me know if my descriptions don't make sense, and I can provide screenshots of what I'm seeing.

srjfoo avatar Jan 08 '24 08:01 srjfoo

This is very cool. It functions the way I was hoping with one bug: I can pan the image past the left, top, and right sides of the viewport but not the bottom.

As Sharon said, that was intentional, (although I didn't realise at the time what scrolling downwards would involve) but it can ~~easily~~ be changed. Perhaps the simplest option would be for the same proportions horizontal or vertical. (edited)

70ray avatar Jan 08 '24 09:01 70ray

There's other thing that I'm not quite sure how to describe.

While experimenting with it I found that if you zoom image 200 or 300% you can't scroll right to the edges. I'm not sure if this is the same problem you were seeing but I'll try to fix it.

70ray avatar Jan 08 '24 09:01 70ray

There's other thing that I'm not quite sure how to describe.

While experimenting with it I found that if you zoom image 200 or 300% you can't scroll right to the edges. I'm not sure if this is the same problem you were seeing but I'll try to fix it.

I didn't try 200-300%, but just fitting the width to the image pane. Under those circumstances, I could scroll right to the edge (top and bottom).

In the clearer light of day, maybe it makes the most sense to just document that the image can be scrolled to a certain percentage of the height if the image is fit to the height of the image pane, and a certain percentage of the width if the image is fit to the width of the image pane, but is proportional under other circumstances.

srjfoo avatar Jan 08 '24 17:01 srjfoo

It should work the same way for horizontal and vertical now and always scroll to the edges: Perhaps it could be described in this way: The image can be scrolled in any direction until only half of it is visible, but if the half is still bigger than the window then it can be scrolled further right to the edge. The "half" is just arbitrarily chosen based on Casey's use case for a text in two columns. It could be changed to some other proportion. The comments in the code suggest how this would work. Sanbox updated.

70ray avatar Jan 09 '24 09:01 70ray

This is closer but I think it needs to account for the zoom level. If I'm zoomed in it still limits panning to the top and the bottom of the image. For testing I went to this project, proofread a page, hit the <-> button to zoom in and then try to pan the image up and down.

cpeel avatar Jan 12 '24 02:01 cpeel

The image can be scrolled in any direction until only half of it is visible, but if the half is still bigger than the window then it can be scrolled further right to the edge.

Horizontal interface:

  1. Fit to width: does not scroll past top and bottom edges of image; left and right scroll is fine.
  2. Fit to height: all scrolling past the edge works as I would expect. I.e. I can see half the image in height and width if I scroll all the way to the top, bottom or either side.

Vertical interface:

  1. Fit to width: works as expected (see point 2 above)
  2. Ditto fit to height.

It just seems to be scrolling past the top and bottom in the horzontal mode that causes the problem. And I'm not quite sure how to describe the circumstances. It seems to have something to do with the height of the image pane as compared with the actual image height? I can't tell for sure; I haven't analyzed it well enough.

srjfoo avatar Jan 12 '24 04:01 srjfoo

It seems to be working as intended. It can be changed to make it scroll until the image is almost off the screen, but why would anyone want to do this?

70ray avatar Jan 12 '24 20:01 70ray

It seems to be working as intended. It can be changed to make it scroll until the image is almost off the screen, but why would anyone want to do this?

In the Standard interface, proofers can dynamically change the height of the image pane, so that if they're proofreading in the horizontal interface, the image height can be narrowed to the point that they only have two or three lines visible, which can help focus if the text is particularly dense. This is not an option in the Enhanced interface, so I can see a proofer wanting to scroll the image such that the top or bottom of the image is as close to the corresponding lines of text as it can get (this would most likely be the top of the page, probably).

Edit: all the rest looks good; I don't think that those need to be changed. The problem is that, in the horizontal interface, scrolling up does not seem to behave as one would expect. It doesn't scroll halfway across the image except when the image is fit to the height of the pane. If the height of the image pane is fairly short, fitting the image to the height of the pane makes it unreadable, and by the time you get enough magnification to be able to read the scan, you can't scroll the image up and down past the limits of the image edges at all. (Left and right, yes, but not up and down): image

Note the position of the scrollbar on the right, and that the top of the image does not scroll below the top of the image, whereas scrolling left and right works fine: image

srjfoo avatar Jan 13 '24 06:01 srjfoo

... so I can see a proofer wanting to scroll the image such that the top or bottom of the image is as close to the corresponding lines of text as it can get (this would most likely be the top of the page, probably).

That seems a good reason. I've changed it so it can scroll to within 60 pixels of the edge. This can be changed easily.

70ray avatar Jan 13 '24 11:01 70ray

Sandbox updated.

70ray avatar Jan 19 '24 16:01 70ray

This is going to be a fairly obvious change -- should it be offered for public review before final approval?

srjfoo avatar Jan 21 '24 09:01 srjfoo

Would it be possible to make the background draggable? That way if the image somehow ended up in a corner the drag target isn't 50x50px box? I suppose you can also use the "fit to x" options too to fix this situation.

chrismiceli avatar Jan 22 '24 16:01 chrismiceli

Would it be possible to make the background draggable? That way if the image somehow ended up in a corner the drag target isn't 50x50px box? I suppose you can also use the "fit to x" options too to fix this situation.

I just poked at it a bit, and I think it's probably quicker and easier to just push one of the buttons that will recenter it. However, that might be a good question to ask potential users.

srjfoo avatar Jan 27 '24 01:01 srjfoo

Would it be possible to make the background draggable? That way if the image somehow ended up in a corner the drag target isn't 50x50px box? I suppose you can also use the "fit to x" options too to fix this situation.

Done that now.

70ray avatar Feb 13 '24 14:02 70ray

The scroll bars are now turned off and drag outside the image enabled as Chris suggested.

Sandbox updated.

70ray avatar Feb 13 '24 14:02 70ray

There is a small blue border in firefox when clicked that probably can be removed: image

I also worry some people may complain there are no scrollbars anymore, so may be worth running by the community.

chrismiceli avatar Feb 13 '24 17:02 chrismiceli

This commit leaves the focus in the text area even when dragging the image. The blue border Chris mentioned indicates that the image has focus so this has now gone.

Sandbox updated.

(The focus() call worked differently in different browsers. This is probably because it is an HTMLelement method and I was calling it incorrectly on a JQuery object)

70ray avatar Feb 15 '24 14:02 70ray

This commit leaves the focus in the text area even when dragging the image. The blue border Chris mentioned indicates that the image has focus so this has now gone.

Sandbox updated.

(The focus() call worked differently in different browsers. This is probably because it is an HTMLelement method and I was calling it incorrectly on a JQuery object)

Thanks for fixing the blue border.

As for restoring focus to textarea after dragging, I'm kind of against that from an accessibility point of view. I don't think we should be doing anything fancy with focus as it could confuse a user who would expect the text area to lose focus when clicking something else. We should let the browser handle focus normally I would think. The current prod behavior also doesn't have this, is it necessary for the image overscroll?

chrismiceli avatar Feb 15 '24 15:02 chrismiceli

This commit leaves the focus in the text area even when dragging the image. The blue border Chris mentioned indicates that the image has focus so this has now gone. Sandbox updated. (The focus() call worked differently in different browsers. This is probably because it is an HTMLelement method and I was calling it incorrectly on a JQuery object)

Thanks for fixing the blue border.

As for restoring focus to textarea after dragging, I'm kind of against that from an accessibility point of view. I don't think we should be doing anything fancy with focus as it could confuse a user who would expect the text area to lose focus when clicking something else. We should let the browser handle focus normally I would think. The current prod behavior also doesn't have this, is it necessary for the image overscroll?

All focus manipulations have been removed by the last commit.

70ray avatar Feb 15 '24 16:02 70ray

This commit leaves the focus in the text area even when dragging the image. The blue border Chris mentioned indicates that the image has focus so this has now gone. Sandbox updated. (The focus() call worked differently in different browsers. This is probably because it is an HTMLelement method and I was calling it incorrectly on a JQuery object)

Thanks for fixing the blue border. As for restoring focus to textarea after dragging, I'm kind of against that from an accessibility point of view. I don't think we should be doing anything fancy with focus as it could confuse a user who would expect the text area to lose focus when clicking something else. We should let the browser handle focus normally I would think. The current prod behavior also doesn't have this, is it necessary for the image overscroll?

All focus manipulations have been removed by the last commit.

Is the sandbox updated? I still see the focus stay in the text area after dragging the image around.

chrismiceli avatar Feb 15 '24 16:02 chrismiceli

Is the sandbox updated? I still see the focus stay in the text area after dragging the image around.

Yes, this seems to be what the users want. I had originally (before this branch) tried to set the focus to the image when clicking on it so that the arrow keys would pan the image. But it seems I did not do it correctly because it only worked like that in some browsers. Users could pan the image by using the scroll bars which did not move the focus out of the text, but now the scrollbars have gone they could no longer do this.

70ray avatar Feb 15 '24 17:02 70ray

Is the sandbox updated? I still see the focus stay in the text area after dragging the image around.

Yes, this seems to be what the users want. I had originally (before this branch) tried to set the focus to the image when clicking on it so that the arrow keys would pan the image. But it seems I did not do it correctly because it only worked like that in some browsers. Users could pan the image by using the scroll bars which did not move the focus out of the text, but now the scrollbars have gone they could no longer do this.

That makes sense, especially the part about dragging with the scroll bars without losing focus now being lost. I just always get worried when manipulating natural events like tab and focus.

chrismiceli avatar Feb 17 '24 00:02 chrismiceli

I poked at the sandbox last night and noticed that the page did not come up in the proofreading interface with focus already in the text. I had to put it there. Is that expected? (Once it was there, it stayed there.) FF, latest version (122.0.1)

srjfoo avatar Feb 17 '24 21:02 srjfoo

I poked at the sandbox last night and noticed that the page did not come up in the proofreading interface with focus already in the text. I had to put it there. Is that expected? (Once it was there, it stayed there.) FF, latest version (122.0.1)

It doesn't in the master branch either (perhaps it did at some time in the past?), but it is possible to make it so it does.

70ray avatar Feb 18 '24 10:02 70ray

Considering the request for just the vertical scrollbar to be retained, the setting style.scrollbarWidth = 'none' applies to both scroll bars. Instead we could set overflow-x: hidden; overflow-y: auto;. This would keep the vertical scroll bar only; the drag would still work and scroll wheel for vertical scroll but shift-scrollwheel would no longer work for horizontal scrolling.

70ray avatar Feb 18 '24 11:02 70ray

Considering the request for just the vertical scrollbar to be retained, the setting style.scrollbarWidth = 'none' applies to both scroll bars. Instead we could set overflow-x: hidden; overflow-y: auto;. This would keep the vertical scroll bar only; the drag would still work and scroll wheel for vertical scroll but shift-scrollwheel would no longer work for horizontal scrolling.

I'm concerned that having only one set of scrollbars would be confusing. The places I see that are literally in places where you can scroll in one direction, but not the other, so I'm inclined to say "all or nothing". (My personal preference is for no scrollbar, honestly. It looks cleaner.

https://caniuse.com/mdn-css_properties_scrollbar-width shows limited coverage for scrollbar-width (which explains why the change didn't work in Opera or Safari), and although it's been in FF since v.64, that's still not as low as our minimum supported browser. And it's only recently supported in Chrome and Edge.

srjfoo avatar Feb 18 '24 16:02 srjfoo