overflow and scrollbars

lfoppiano / streamlit-pdf-viewer

Streamlit PDF viewer

https://structure-vision.streamlit.app/

Apache License 2.0

62 stars 3 forks source link

overflow and scrollbars #8

Closed lfoppiano closed 5 months ago

lfoppiano commented 7 months ago

When the component is loaded, it pushes the scrolling back down. We should add an option that allow the component to have a overflow:scrolling.

Tasks

[x] The user does not set any parameters (e.g. pdf_viewer(file)) -> we unwrap the PDF (no scroll bars, every page one after the other)
[x] The user does set height = 700 (absolute number) -> then we show the scrollbars and limit the window to the height
[x] ~~The user does set height = 80% (relative number) -> same as previous, show the scrollbars and limit the window to the height~~ because streamlet.setFrameHeight doesn't support relative number

t29mato commented 7 months ago

https://github.com/lfoppiano/streamlit-pdf-viewer/assets/30012556/fe83b153-bb8a-4e99-8c16-07324a68b0c0

lfoppiano commented 7 months ago

looks good.

We want to have both the previous and this behaviour, selected by parameters, I think when the user (User = anybody/developers that uses the component) passes a height value:

The user does not set any parameters (e.g. pdf_viewer(file)) -> we unwrap the PDF (no scroll bars, every page one after the other)
The user does set height = 700 (absolute number) -> then we show the scrollbars and limit the window to the height
~~The user does set height = 80% (relative number) -> same as previous, show the scrollbars and limit the window to the height~~

t29mato commented 5 months ago

The user does set height = 80% (relative number) -> same as previous, show the scrollbars and limit the window to the height

The argument of Streamlit.setFrameHeight() supports only numbers, not str like 80%, so to support this feature, we need to calculate from the user's window size, which is not sure it can make it.

The user does not set any parameters (e.g. pdf_viewer(file)) -> we unwrap the PDF (no scroll bars, every page one after the other)

I can make it. it would take 1 - 4 hours.

lfoppiano commented 5 months ago

The argument of Streamlit.setFrameHeight() supports only numbers, not str like 80%, so to support this feature, we need to calculate from the user's window size, which is not sure it can make it.

Let's drop it then, I can validate the parameter in the python part.

I can make it. it would take 1 - 4 hours.

Isn't it already the current behaviour?

t29mato commented 5 months ago

I can make it. it would take 1 - 4 hours.

Isn't it already the current behaviour?

Yeah, that's true. now, it's handling both cases

https://github.com/lfoppiano/streamlit-pdf-viewer/commit/9196b8509bf2f285062af0eff71799037d3d7be4

lfoppiano commented 5 months ago

@t29mato could you please summarise what exactly should be tested? The point number 2 is it implemented? Is this implemented in the main or as a PR?

t29mato commented 5 months ago

summarise what exactly should be tested?

Test for Unset height (Default Behavior):
- When height is not set (i.e., None), the PDF should display in its entirety without any overflow.
Test for Set height (Numeric Value):
- If a numeric value is assigned to height, the PDF should display with the specified height in pixels, and overflow should be enabled.
Test for Invalid height Input:
- The function should throw an error if height is given a value other than a number or None.

The point number 2 is it implemented?

Is point number 2 The user does set height = 700 (absolute number)? Then, yes, I've implemented it.

Is this implemented in the main or as a PR?

Sorry, I don't understand this.