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

Invalid canvas size #54

Closed
lfoppiano opened this issue Jul 20, 2024 · 15 comments · Fixed by #58
Closed

Invalid canvas size #54

lfoppiano opened this issue Jul 20, 2024 · 15 comments · Fixed by #58
Labels
bug Something isn't working implemented
Milestone

Comments

@lfoppiano
Copy link
Owner

lfoppiano commented Jul 20, 2024

Using this PDF in here lead to "invalid canvas size" popup.

@lfoppiano lfoppiano added the bug Something isn't working label Jul 20, 2024
@lfoppiano
Copy link
Owner Author

lfoppiano commented Jul 25, 2024

@SiddhantSadangi I did some tests and the issue seems to be related to the fact that you're calling the pdf_viewer multiple time while loading the PDF.

I did not find why exactly why this is happening yet, I'm still investigating and is quite time consuming.
Anyway I think the issue is most likely that we never designed this component to have multiple instance of the PDF opened, so we need to generate some unique id to be used for the name of certain internal HTML elements.
Looking better, each component should be isolated by an iframe so I don't really know what happens and why we have this invalid frame size

@lfoppiano lfoppiano changed the title invalid canvas size Invalid canvas size Jul 25, 2024
@SiddhantSadangi
Copy link

SiddhantSadangi commented Jul 25, 2024

Thanks for looking into this @lfoppiano .

If it was an underlying issue with the component itself, the popup would show for any PDF.
Could there be something specifically in this PDF that is breaking the component?

@lfoppiano
Copy link
Owner Author

lfoppiano commented Jul 25, 2024

@SiddhantSadangi indeed, that's a good point. if I remove the alert after catching an exception, it will works as if nothing have happened, showing all the PDF correctly, but I want to do a few more tests before that.

@lfoppiano lfoppiano added this to the 0.0.15 milestone Jul 30, 2024
@ehenry09
Copy link

I have been having this same issue. I thought it was the PDF size, but it still appears to be happening when I reduce the width to something very small.

I appreciate the support on this!

@lfoppiano
Copy link
Owner Author

@ehenry09 could you share the PDF and the code that you used (or a similar example)?

@ehenry09
Copy link

ehenry09 commented Jul 31, 2024

@ehenry09 could you share the PDF and the code that you used (or a similar example)?

I played around with this a little bit this morning, and I ** think ** I am only getting this error when I try to render a PDF in the context of a tab.

This is the PDF file: https://www.dropbox.com/sh/wty3w9kl7v8u40t/AADbg8ITc8ZUpcatDAaLB-8sa/Current%20Rule%20Book/1.2%20Final?dl=0&preview=Rulebook+1p2+Final+Version+02-27-23+Optimized.pdf&subfolder_nav_tracking=1

Here is an example of code that I used to reproduce it.

import streamlit as st
from streamlit_pdf_viewer import pdf_viewer


def init_session_state() -> None:

    if "curr_page" not in st.session_state:
        st.session_state["curr_page"] = 1


def set_curr_page():
    try:
        if st.session_state["user_page_input"].isdigit() and int(st.session_state["user_page_input"]) > 0:
            st.session_state["curr_page"] = st.session_state["user_page_input"]
    except:
        pass


def render_rulebook_tab() -> None:

    min_page = 1
    max_page = 1000

    col1, col2, col3 = st.columns([4, 2, 2])

    with col1:

        user_input = st.text_input(
            label="Rulebook Navigation",
            value=None,
            on_change=set_curr_page,
            placeholder="Go to page ...",
        )

    with col2:

        previous = st.button(
            label="Previous",
            key="previous",
            use_container_width=True,
        )

    with col3:

        next = st.button(
            label="Next",
            key="next",
            use_container_width=True,
        )

    if previous:
        st.session_state["curr_page"] = max(min_page, int(st.session_state["curr_page"]) - 1)

    if next:
        st.session_state["curr_page"] = min(max_page, int(st.session_state["curr_page"]) + 1)

    if st.session_state["curr_page"]:
        pdf_viewer("data/rulebooks/middara/Rulebook 1p2 Final Version 02-27-23 Optimized.pdf", pages_to_render=[int(st.session_state["curr_page"])])



def render_main_page() -> None:
        tab1, tab2 = st.tabs(["tab1", "tab2"])

        with tab1:
            st.markdown("Test Tab")

        with tab2:
            render_rulebook_tab()


def main() -> None:

    init_session_state()
    render_main_page()


if __name__ == "__main__":
    main()

@lfoppiano
Copy link
Owner Author

This issue should be solved in #58. I tested with the PDFWorkbench in local, and it seems fine, but there might be other bugs around the corner.

I needed to change the behavior, now I don't try to occupy all the space, but I try to keep the proportions on the PDF.

It would be good if someone could test it before I make the release. The process is not too complicated but may take some steps:

  • git clone https://github.com/lfoppiano/streamlit-pdf-viewer
  • git checkout bugfix/rendering-and-width
  • cd streamlit_pdf_viewer/frontend
  • npm run build
  • cd /yourproject
  • pip install -e path_of_the_pdfviewer

Let me know
Luca

@SiddhantSadangi
Copy link

SiddhantSadangi commented Aug 1, 2024

@lfoppiano - I am on Windows (yes, I know) and don't have npm installed. Will be happy to test if you can push a pre-release version to PyPI :)

@lfoppiano
Copy link
Owner Author

Thanks @SiddhantSadangi no problem. Let's see if someone else can test it, meanwhile I need to fix the tests and add a few more before I can attempt a pre-release. I keep you posted.

@ehenry09
Copy link

ehenry09 commented Aug 2, 2024

I am happy to test, though am on vacation until next week. I am also running in a docket container, not sure if that impacts how I should test it out.

@lfoppiano
Copy link
Owner Author

I've released the version 0.0.15. It's not yet officially published, but it's on pypi. Could you please give it a try?

@SiddhantSadangi
Copy link

@lfoppiano - I just tested 0.0.16, and it works great! Thanks 🎉

Please let us know once a stable version is released on PyPI

@ehenry09
Copy link

ehenry09 commented Aug 5, 2024

I just tested it as well. Thanks for the update :D

Will let you know if I have any further issues.

@lfoppiano
Copy link
Owner Author

The new version is released. If works fine feel free to close this issue.

Thanks!

@SiddhantSadangi
Copy link

@lfoppiano - works fine for me, thanks a ton! 🫶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants