search

keyboardio / Chrysalis

Graphical configurator for Kaleidoscope-powered keyboards
https://github.com/keyboardio/Chrysalis#chrysalis
GNU General Public License v3.0
497 stars 64 forks source link

Don't print AppBar and tooltips on layout cards #1180

Closed EvyBongers closed 1 year ago

algernon commented 1 year ago

I think the snackbar hiding is fine, but hiding the AppBar should only be done on the layout card screen, not anywhere else. The AppBar is - in my opinion - useful to have when printing (usually to a file) other screens. Simply hiding the AppBar is also insufficient: if you have the main menu drawer on the left open, hiding the appbar will not hide the drawer. This is why #1020 opted to hide the whole header (drawer included) from JS, rather than through a media query.

Is the AppBar hiding in CSS necessary? Does Chrysalis fail to hide the header in print for layout cards? (on my Linux, printing the layout card screen correctly hides the header)

EvyBongers commented 1 year ago

I think the snackbar hiding is fine, but hiding the AppBar should only be done on the layout card screen, not anywhere else. The AppBar is - in my opinion - useful to have when printing (usually to a file) other screens. Simply hiding the AppBar is also insufficient: if you have the main menu drawer on the left open, hiding the appbar will not hide the drawer. This is why #1020 opted to hide the whole header (drawer included) from JS, rather than through a media query.

I'm happy to look into that.

Is the AppBar hiding in CSS necessary? Does Chrysalis fail to hide the header in print for layout cards? (on my Linux, printing the layout card screen correctly hides the header)

I regularly print layout cards and the app bar always shows for me (Arch, AppImage). Could it be different based on how the printing is triggered (print button vs keyboard shortcut)?

algernon commented 1 year ago

I regularly print layout cards and the app bar always shows for me (Arch, AppImage). Could it be different based on how the printing is triggered (print button vs keyboard shortcut)?

Bingo! I was using the shortcut, and had no header there. Just tried with the button, and the header is printed. Good catch! The fix then should be making sure the button does the same things as the shortcut.

EvyBongers commented 1 year ago

So, I just removed the css hiding the header, leaving it to the hook added in #1020 to do this. However, when using the print button, it looks like the document is rerendered again before the print is actually sent to the printer, where the useMediaQuery("print") function return false again.

As useMediaQuery seems to be React's answer to dynamically adding/removing media selectors, I'm tempted to believe this is an issue with react, rather than how Chrysalis implements it. That having said, I only have limited experience with web and even less with React, so please correct me if I'm wrong.