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

Error message #41

Closed
flachware opened this issue Jun 10, 2020 · 6 comments
Closed

Error message #41

flachware opened this issue Jun 10, 2020 · 6 comments

Comments

@flachware
Copy link
Contributor

@dstillman @mrtcode when a PDF file is corrupted, can we detect that before opening a new window/tab and show an error message in the main window?

@mrtcode
Copy link
Member

mrtcode commented Jun 10, 2020

pdf-worker can detect if a PDF file is corrupted, but it has to firstly process it which may take time (especially if there are many new PDFs added). So, no, we can't completely prevent user from trying to open corrupted PDF files.

@flachware
Copy link
Contributor Author

Ok, this means we need an error message in the PDF window/tab, right? How do error messages look like in the client, is it an error dialog, an alert bar, …? Is there a way I can spoof an error in the client so I can have a look? (I’ve just seen the retraction watch bar so far.)

PDF.js includes an error message, a bar, and I’m wondering if we can make use of it (and if we have to at all, will error notifications even be part of the pdf-reader?). The way the pdf-reader layout works that error message would be absolutely positioned on top of the layout (covering at least some part of the UI). If we wanted to integrate it more neatly we would probably need to calculate the current size of the bar (on window resize) and use this information to adjust the layout so there is no overlap.

Screenshot 2020-06-10 at 17 43 03

@dstillman
Copy link
Member

We use real-ish dialogs in the client (e.g., press Delete on a collection), but a corrupted PDF is probably rare enough that we can just do this within the window with a bar, and then we won't have to implement it separately for the web.

@flachware
Copy link
Contributor Author

Well I have been wondering if there was a way to not show a new window/tab in the case of a corrupted PDF. If we show a bar with a close button in the pdf-reader, the user would basically close the bar and/or close the window/tab, but there is not much point in having a new window/tab in the first place (except we need it to show the loading progress), when the only thing you can do is to close it.

So if we need a new window/tab, maybe we could close it and show the error dialog in the main window? In the web-library we already have a generic system for different kinds of notifications, I think we would show error messages there, not in a new type of bar. I’m trying to make sense of the different kinds of dialogs/notifications/bars, so we don’t implement the same functionality twice or even more often. We also have already the sticky positioned import bar in the pdf-reader, in terms of layout it would be the better place to show errors, at least when we use it for errors other than corrupted PDFs.

@dstillman
Copy link
Member

I think closing the window automatically and popping up another error dialog would be more jarring than just showing an error in the PDF reader window. And again, this should be rare enough that the user experience doesn't really matter — if they have to click close on a pointless window/tab, that's fine. I wouldn't spend more than a couple minutes styling pdf.js's existing error handling for this.

@flachware
Copy link
Contributor Author

Ok, so I’ll use the existing error bar in pdf.js.

flachware added a commit that referenced this issue Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants