PaperUI: rework bindings list view & binding detail view #3782
Conversation
I think if people work on other peoples PRs it would be a good idea to make a comment so we don't all waste time working on the same thing...... Anyway... So you have now removed the extension panel for the overview? What happens if you have a large overview - wasn't this the purpose of the extension panels? I personally don't like the borders around the supported things list, but that's just personal preference maybe. Otherwise looks fine to me. Shall I close the other PR? |
It was actually me that did the comment on #3237: "I'll try to come up with some concrete changes"
No, see #3237 (comment). Excessive documentation should rather be put on the official docs page (the README.md of the binding).
I assumed that this is a fixed design of the extension panels. If it is possible to change, I would also prefer a different, more Material-Design-like design. |
Just noticed that this sentence does not pay the due respect. It was actually him, who had the good ideas how to get the design of all our different pages and lists to a common approach to have an overall better UX. I, as usual, only asked stupid questions inbetween :-) |
and
What do you mean by "border" and "more MD-like"? In the guidelines https://material.io/guidelines/components/expansion-panels.html# the expansion panel is always bordered. If you refer to the whitespace left and right to the panel its due to the fact that the whole content in the binding details view is surrounded by a border. If we change the expansion panel to float from left to right w/o borders it will break the whole column with overview and "Supported Things" sub-headline.
not at all ;-) |
Ok, I just comment that it would be useful to say that someone else is working on a PR as I also have spent some more time on this while waiting for feedback to my questions and I just prefer to avoid wasting time if someone else has take over the development. |
@cdjackson agreed. we started the discussion when your PR was closed and in the beginning it was just meant to be a prototype. Then the scope got too big to make a PR against your work. Next time I will leave you a message. |
Ok, I will close my PR. |
But note that it is less than 24 hours ago that all of that was done - so a message wouldn't have changed much. So @cdjackson, if you have pending work on PRs that isn't publicly visible yet, better leave a message ;-) |
Looking at your link, I would say that if we changed the background from white to light-gray, it would perfectly match the MD-examples. I am aware that this comment might cause quite some further big refactorings, though 😎 |
I´m fine with that 👍 |
Sure, a follow-up PR will do! |
Visually, this PR then looks good for me, wrt the code, I would leave the review to someone else, though. |
Ok, but for me the person creating the PR should retain the baseline unless discussion presents that someoene else will take over or something. Maybe here it's a bit messy as I accidentally closed the PR, but I made changes a week back as per my questions to you and there was no response until this PR. I don't meant to be a pain, but I also don't want to waste my time, or anyone elses.... |
Clearly nobody wants, so accept my apologies if that has happened. Instead of requiring more of your time by making fuzzy comments, I through it would help to come around with real code as a PR against your PR to address my concerns - I would consider this a usual practice. |
That's fine, but I just ask that someone make a comment so that we don't end up with multiple people working on the same PR as we have here. You guys clearly benefit from other communications channels so I just ask that if someone takes over someone elses PR that there's a comment/discussion at least. |
As I said above: I HAD left a comment and promised to come up with something - so I assumed the ball was on my side. I'll try to express myself clearer the next time! |
It was not meant a takeover, but a contribution to it, which became bigger and bigger. |
Also: refactor list views Also-by: Chris Jackson <chris@cd-jackson.com> Signed-off-by: Henning Treu <henning.treu@telekom.de>
@cdjackson I know the timing and communication was not ideal, but do you mind taking a closer look on this one? Would be really nice. |
@@ -12,6 +12,7 @@ | |||
"angular-animate": "1.4.8", | |||
"angular-aria": "1.4.8", | |||
"angular-material": "1.0.1", | |||
"angular-material-expansion-panel": "^0.7.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that for stuff that we package we have to stick to the exact version, only for build&test dependencies like gulp we can go higher.
…dencies to devDependencies Signed-off-by: Henning Treu <henning.treu@telekom.de>
done. I did not touch See #3889. |
Thanks! Imho good to merge. |
-> #3893 |
Also: refactor list views.
@cdjackson I took you work from #3237 and reworked the bindings view a little. The expansion panels are now used for the thing types and the bindings are listed instead of cards. Let me know what you think.
Here are some screenshots:
Binding list:
Sonos binding, editable and more then three supported things with filter:
Sonos binding with filter applied:
Astro binding w/o configuration and filter: