gismo141 / homebridge-server

Server plugin for homebridge
https://gismo141.github.io/configure-your-homebridge-2/
152 stars 20 forks source link

Wraps stringified JSON in HTML pre tags without .replace #27

Closed Danjohnsonnj closed 7 years ago

Danjohnsonnj commented 7 years ago

To make it easier to read (and potentially to copy, depending on the browser/system), just output the JSON as-is wrapped in pre tags, without the string .replace method for the br tags.

Danjohnsonnj commented 7 years ago

I've found that with longer values, the entire table overflows its parent, causing a scroll. This scroll could be non-obvious when the user's system settings hide scroll bars unless actively scrolling (default on macOS now). This can end up hiding the remove icon beyond the parent element.

Looking into either getting only the appropriate TD to overflow with scroll, or rewriting in another manner to address this.

gismo141 commented 7 years ago

Yeah I already have a look at it and found that issue as well - otherwise very good input!

Maybe this could be like a expand on hover snippet? My dream would otherwise be to have a complete editable and dynamic tree for the additional JSON-data but I haven't found a satisfying solution yet.

Danjohnsonnj commented 7 years ago

I'm playing with the idea that the markup remains a table set to display block, but various children will have their CSS display properties as either inline-flex (th, td) or flex (tr). Combining this with appropriate widths and overflow properties, it lays out nicely. I would recommend a separate local CSS file that is included via index.js.

There's very wide support for flex (http://caniuse.com/#search=flex) but if the browser doesn't understand it, it should fall back to the current table rendering! Best of both worlds.

I'm not sure about an expand on hover. While testing out some solutions, I realized it doesn't help on smaller screens and large properties. My Nest token, for example, is almost 150 characters long! My main goal was to ensure the remove icon was always visible, so I think it's somewhat of an improvement.

Since I wasn't able to get an external local CSS file to load properly, I added it as a new inline stylesheet getting added into the HEAD, as others already were. For clarity I separated the declarations onto individual lines, but that's really just to make debugging/adjustments easier for now.

Thanks!

Danjohnsonnj commented 7 years ago

homebridge-server-desktop homebridge-server-mobile

Danjohnsonnj commented 7 years ago

Oh gosh. Horrible in Safari. I'll take a look and do some cross browser testing, as I should have. :embarrassed:

Danjohnsonnj commented 7 years ago

I'm going to do this in two different PRs, both adding the pre and removing the .replace, bot one will adjust table styles and the other is all divs and table-less. It's not semantic, but fighting table styles is no fun :)

Thanks for your work, this is a pretty cool project!

gismo141 commented 7 years ago

Sounds great - I really appreciate your help!