nextcloud / tables

🍱 Nextcloud tables app
https://apps.nextcloud.com/apps/tables
GNU Affero General Public License v3.0
148 stars 23 forks source link

add print styles #931

Closed FahrJo closed 7 months ago

FahrJo commented 7 months ago

This PR addresses https://github.com/nextcloud/tables/issues/107. It was tested with Firefox 123.0.1, Chromium 116.0 and Safari 17.3. There sometimes occur minor glitches on Chromium and Safari but Firefox works well with my sample tables and views.

enjeck commented 7 months ago

Hi @FahrJo. Can you please explain what this fixes? There are no details at https://github.com/nextcloud/tables/issues/107 either. If possible, please attach before/after images

FahrJo commented 7 months ago

Hi @enjeck, of course I can explain. Previously, the print view included the side bar as well as some control elements. Additionally only the first page was filled and not scaled to the size of the media at all.

Firefox 123.0.1 print dialog before the fix Firefox 123.0.1 print dialog after the fix

I also attached some sample printouts as PDF (yes, the Safari output was blank before the fix). The glitches I mentioned above are the untidy row breaks at the page breaks in the Chromium pdfs for instance (the collectives app has the same issues with page breaks btw.).

tables_print_chromium116_after_view.pdf tables_print_chromium116_after.pdf tables_print_chromium116_before_view.pdf tables_print_chromium116_before.pdf tables_print_firefox123_after_view.pdf tables_print_firefox123_after.pdf tables_print_firefox123_before_view.pdf tables_print_firefox123_before.pdf tables_print_safari17_after_view.pdf tables_print_safari17_after.pdf tables_print_safari17_before_view.pdf tables_print_safari17_before.pdf

The views are exported as landscaped page to demonstrate the scaling and appearance for wide tables. Tell me if you need more demo material.

FahrJo commented 7 months ago

Yes, that is the expected behavior with the default CSS but in common use cases, one only wants to print the table without any space lost for useless printed controls and menu sections. The collectives app is a great example for that same situation and already implements this. Also on many news pages or Wikipedia for example, if you print the page, the view is reduced to the main content and drops the GUI controls, adds, recommendations and comment sections.

I also add an example for a wide table:

Wide table print dialog

The content scales to the size of the page but I additionally adjusted the scaling setting in the print settings to avoid a line break. But there are some limits for content on a page for sure. In that situations (very wide tables) I would propose reducing the columns to print in a separate view of the table.

Veery wide table print dialog
elzody commented 7 months ago

I think the most expected behavior can be subjective, but when it comes to printing I think some UI elements should definitely be clipped. It's hard to say what a user might expect when they click the print button - maybe it's a matter of keeping the printing as-is when clicking the print button in the browser, but adding a print button to Tables could provide a way to print exclusively the table when desired, or tweak the behavior even further.

It may be difficult to come up with a solution for wide tables/columns, but I think the changes so far look okay. May require some further discussion, but I think you're on the right track.

enjeck commented 7 months ago

@FahrJo Please run npm run stylelint:fix and npm run lint:fix and commit. That should fix the stylelint CI checks.

FahrJo commented 7 months ago

I agree that there is room for improvement, but so far printing a table has not been possible at all. If I have the time, I can also add a button for this print view (in another PR). On the other hand, that would require consideration of the keyboard shortcut Ctrl+P. As a user, I would personally prefer the default print content to be the table/view only.

elzody commented 7 months ago

I agree that there is room for improvement, but so far printing a table has not been possible at all. If I have the time, I can also add a button for this print view (in another PR). On the other hand, that would require consideration of the keyboard shortcut Ctrl+P. As a user, I would personally prefer the default print content to be the table/view only.

That definitely makes sense. Couldn't hurt if this is something you feel like implementing would be good. If anything it would be nice to bring this issue to light some more, since I think it's important to consider. It's only natural some users would want to print tables, so defining the behavior of this is good

github-actions[bot] commented 7 months ago

Hello there, Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!