[FancyZones] Do not zone window if it merges with other window #6549
Conversation
…tab into other Chrome window
| auto myTie = [](const FancyZonesUtils::FancyZonesWindowInfo& windowInfo) { | ||
| return std::tie(windowInfo.noVisibleOwner, windowInfo.processPath, windowInfo.standardWindow); | ||
| }; | ||
| if (myTie(windowInfo) == myTie(m_moveSizeStartWindowInfo)) |
There was a problem hiding this comment.
This might be fine, but I would feel more comfortable if we do an explicit check for the specific case we know of and only bail out only if windowInfo.noVisibleOwner and windowInfo.standardWindow change from true to false.
Also, let's not check windowInfo.processPath because as far as we know, this is not a factor.
Also add a comment explaining what case we are covering with that check.
| zoneWindow->MoveSizeEnd(m_windowMoveSize, ptScreen); | ||
| auto windowInfo = FancyZonesUtils::GetFancyZonesWindowInfo(window); | ||
| auto myTie = [](const FancyZonesUtils::FancyZonesWindowInfo& windowInfo) { | ||
| return std::tie(windowInfo.noVisibleOwner, windowInfo.processPath, windowInfo.standardWindow); |
There was a problem hiding this comment.
I like what You did here, but I still think that overriding == operator would be cleaner approach. But I’m perfectly fine with both. Nice work!
| if (windowInfo.standardWindow == m_moveSizeStartWindowInfo.standardWindow && | ||
| windowInfo.noVisibleOwner == m_moveSizeStartWindowInfo.noVisibleOwner) |
There was a problem hiding this comment.
Let's do:
if (windowInfo.standardWindow == false && windowInfo.noVisibleOwner == false && m_moveSizeStartWindowInfo.standardWindow == true && m_moveSizeStartWindowInfo.noVisibleOwner == true)
{
// Abort the zoning, this is a Chromium based tab that is merged back with an existing window
...
}
So the check is 100% specific for the Chrome tab case and doesn't cover the other way around (that we don't even know if it's actually possible, but if it is, it's not currently broken)
There was a problem hiding this comment.
other way around isn't possible, as if standardWindow was false in moveSizeStart, we wouldn't get here at all
There was a problem hiding this comment.
Still, I prefer the code covers the specific case, this is for long term code maintainability, it should only check what we know it should check and not cover other cases.
e.g. merge Chrome tab into other Chrome window
Summary of the Pull Request
What is this about?
If some of the window properties changed while drag&drop (durring MoveSize* FZ methods), that means that window is merged with other window or changed in some other way. Therefore, that window shouldn't be zoned.
PR Checklist