--it creates a much better and consistent visual effect
when applets pop ups that are present in a dock draw
all their borders.
Details
Details
- Reviewers
broulik davidedmundson kossebau - Group Reviewers
Plasma VDG
--use Latte dock in order to confirm the behavior
--use traditional plasma panel in order to confirm
that nothing broke
Diff Detail
Diff Detail
- Repository
- R242 Plasma Framework (Library)
- Branch
- dockBorders
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 3297 Build 3315: arc lint + arc unit
Comment Actions
A big improvement! Even better would be a little downward-pointing triangle pointing to the thing that spawned it. Here's a visual example from macOS with a vertical Dock:
Comment Actions
The idea is fine in principle but Plasma may also set a mask on its panels, noticeably in non-composited case for rounded corners, so when I disable compositing I get borders on all side for regular panel popups
Comment Actions
I like both ideas so far. Great improvement! Would it be too hard to create the pointer triangle? I think it make so much sense. That way the poppup doesn't feel like it is floating out of nowhere.
Comment Actions
A big improvement! Even better would be a little downward-pointing triangle pointing to the thing that spawned it. Here's a visual example from macOS with a vertical Dock:
I am not sure.
Latte is trying to be as much plasma as possible and a "downward-pointing triangle" isnt shown any where else in plasma.
So that would make Latte to look a bit out of place in Plasma.
I think for now a small improvement would be enough.
Comment Actions
A downward-pointing triangle doesn't make sense for pop-ups with a standard Plasma panel because pop-ups appear to slide out of panels, and they're always touching one another. So there is never a question about what the parent is. Latte has a different visual style and pop-ups are sort of floating in space, which is why I suggested the triangle.
Comment Actions
A big improvement! Even better would be a little downward-pointing triangle pointing to the thing that spawned it. Here's a visual example from macOS with a vertical Dock:
I am not sure.
Latte is trying to be as much plasma as possible and a "downward-pointing triangle" isnt shown any where else in plasma.
So that would make Latte to look a bit out of place in Plasma.
I think for now a small improvement would be enough.
Our HIGs don't prevent it. It is also a different element than a popup or sliding notification.
Comment Actions
@ngraham @abetts I searched a bit the "downward-pointing triangle" case but I dont think it can be drawn based on the current plasma theme implementation. Plasma themes are svg(s) so the borders images can also have borders so it isnt just drawing a triangle based on the theme background color.
I believe it would be better if the relevant discussion could be taken place at another place, in order to keep this review clean for feedback.
This commit improves the situation for Latte a lot until something better is supported.
The arrow case will also need full borders in order to be drawn correctly so this commit still has meaning.
| src/plasmaquick/dialog.cpp | ||
|---|---|---|
| 209 | How does parentWindow->mask relate to being a dock or not? Also we need to be a bit careful on names. | |
| src/plasmaquick/dialog.cpp | ||
|---|---|---|
| 209 | The idea was first introduced at popupPlacement() in dialog.cpp. It is based on the assumption that by design plasma panels do not use mask() because their shadows are drawn external from the window and most of the docks (e.g. Plank,Latte) are using masking for almost all of their operations. Do you believe there is a better way to distinguish between docks and panels ? | |
Comment Actions
It is based on the assumption that by design plasma panels do not use mask()
It's a big assumption. It seems more based on what the two windows happen to currently do rather than anything related to the topic at hand.
Especially problematic as it's in frameworks we can't assume our code is the only user.
first introduced at popupPlacement()
...by you :P
Extra confusingly the comment above that line is from before the mask test was added, and in that comment dock means panel.
So the comment about when we're a dock explicitly excludes what you call a dock. :/
Do you believe there is a better way to distinguish between docks and panels ?
Implicitly? Probably not.
I would have pushed for doing it explicitly. Ideally when outsideParentWindow was first introduced.
Given we're using this mask test already, I don't like it, but if no-one else objects, ship it.
(please also clarify comments here and line 906 to distinguish dock and dock)
Note, when Dialog gets a rewrite, we won't do it like that. I want a hint at the containment level that we can forward through CompactApplet.qml
Comment Actions
I know :( and this broke plasma popups placement in !compositing mode.
This is why I sent https://phabricator.kde.org/D15821
which uses mask() in a more meaningful way both for plasma and Latte.
mask() in D15821 isnt used to distinguish plasma/docks but to place
popups more intellengently. But in D15821 I need a window flag in order
to identify which popups can be placed according to the element trigerred them.
I used Qt::Tooltip for this in D15821 but I dont like it, I would prefer something
which is not used that often.
Extra confusingly the comment above that line is from before the mask test was added, and in that comment dock means panel.
So the comment about when we're a dock explicitly excludes what you call a dock. :/I would have pushed for doing it explicitly. Ideally when outsideParentWindow was first introduced.
sorry, I dont understand what explicitly means
Given we're using this mask test already, I don't like it, but if no-one else objects, ship it.
(please also clarify comments here and line 906 to distinguish dock and dock)
maybe is better to look first at D15821 and decide afterwards if this should be commited now or when dialog2 arrives
Note, when Dialog gets a rewrite, we won't do it like that. I want a hint at the containment level that we can forward through CompactApplet.qml
no problem for me, we can wait for this...
Comment Actions
sorry, I dont understand what explicitly means
As in "void setAlwaysShowAllBorders(bool)" (or whatever) that a user calls.
Comment Actions
Or when popups width is bigger than the non-full-width panel from which they are triggered (see also "full-screen" app window challenged the same time):
Actually popups from Plasma panels itself, at least with non-full-width-non-hide ones, now & then could use some more borders as well.
E.g. when popup is shown next to screen edge with non-full-width panel, where only parts of popup are touching the panels, other parts the void:
Or when popups width is bigger than the non-full-width panel from which they are triggered (see also "full-screen" app window challenged the same time):
Also does Plasma assume panels have simple straight edges which one or the other theme might not comply with ;)
And I fully agree that some visual indicator to the origin of a popup, like the triangle, might improve experience a lot. IMHO a must for future Plasma styles :)
Not only because the position of a popup can not always be aligned with the origin item due to place constraints.
So, IMHO it would be great to have UI/UX designers go to their drawing boards and define some look & feel rules based on this. Which work for Plasma panel ideas and Latte dock ideas.
Actually, also for "normal" programs while on it, as normal popup dialogs or content-shifting inline dialogs annoy me a lot :)
(while on it, flagging this as Change Requested officially, given the !mask does not work as we all found)
Comment Actions
No time for Plasma in the next months reserved, so withdrawing as reviewer/commenter to clear my todo list