The popup/exec if was duplicated three times, only to calculate the y position.
- Group Reviewers
- R108:0a5b029a391f: Reduce duplicate code calculating popup position
Hi @romangg, I did not test this. I added inline comments explaining why the code is completely unchanged. It does 100% the same as before, just without the duplication. All you need is to invert the two conditions inside the ifs and switch the else branch with the first branch of the if and you'll arrive at this.
If (y == pos.top()) we use the x, y position unchanged (I'm unsure why this special case is necessary, it skips the caclulations below, so it's a micro-optimization).
This bit was executed after the first if was !=. Unchanged.
>= is the inversion of <, so this corresponds to the second, inner else. It is the only case where x, y are modified - y is calculated.
I'm a bit unsure what you mean - the entire popup position trickery? I was wondering how much it's needed, but didn't want to break existing things. For now I assume the code is there for a reason - it seems to me that it want to move the popup high enough to not be positioned at the bottom in a way that it would be cut off. I'm not sure what Qt actually does... in theory I think it should move it up.
I don't see why we have to ensure that QMenu doesn't fall off screen. QMenu already does that for us. Besides that our code is incomplete(only one out of three cases is handled) and incorrect(y + popupHeight >= area.height() is wrong).
Remove if (y != pos.top()) and everything that goes inside.