D16325: [wayland] XdgPopup Positioning

zzag added a comment.


  Some minor nitpicks.

INLINE COMMENTS

> abstract_client.cpp:1175
> +    Q_UNUSED(bounds);
> +    Q_ASSERT(false);
> +    return QRect();

Q_UNREACHABLE.

> abstract_client.h:395
>      /**
> -     * @returns The recommended position of the transient in parent coordinates
> +     * Only valid is hasTransientPlacementHint is true
> +     * @returns The position the transient wishes to position itself

Typo: "only valid is"

> kwin_wayland_test.h:27
>  #include <QtTest>
> +#include <KWayland/Client/xdgshell.h>
>  

That's not Qt include. :-)

> transient_placement.cpp:310
> +    // ----------------------------------------------------------------
> +    // contrain and flip
> +    positioner.setConstraints(XdgPositioner::Constraint::FlipX | XdgPositioner::Constraint::FlipY);

Typo: contrain.

> placement.cpp:505-509
> +    // potentially a client could set no constraint adjustments
> +    // and we'll be offscreen
> +    // The spec implies we should place window the offscreen
> +    // but practically Qt doesn't set any constraint adjustments yet so we can't.
> +    // Also it's not like kwin to let clients do what they want

Please reformat it.

> shell_client.cpp:1619
> +        gravity = m_xdgShellPopup->gravity();
> +        offset= m_xdgShellPopup->anchorOffset();
> +        constraintAdjustments = m_xdgShellPopup->constraintAdjustments();

Missing whitespace before "="

> shell_client.cpp:1622
> +    } else {
> +        Q_ASSERT(false);
>      }

Q_UNREACHABLE

> shell_client.cpp:1704
> +    QPoint anchorPoint;
> +    switch(anchorEdge & (Qt::LeftEdge | Qt::RightEdge)) {
> +    case Qt::LeftEdge:

Missing whitespace between "switch" and "("

> shell_client.h:213
>      void updatePendingGeometry();
> +    QPoint popupOffset(const QRect anchorRect, const Qt::Edges anchorEdge, const Qt::Edges gravity) const;
>      static void deleteClient(ShellClient *c);

We probably don't need const for anchor edges and gravity.

REPOSITORY
  R108 KWin

BRANCH
  xdg_popup

REVISION DETAIL
  https://phabricator.kde.org/D16325

To: davidedmundson, #kwin, graesslin
Cc: zzag, kwin, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart