From 128abc692e91498bec65bceb71a83c255402ddb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Lucas=20Golini?= Date: Fri, 18 Sep 2026 13:59:11 -0300 Subject: [PATCH] Fix UIDropDown listbox mis-positioning. Fix sort indicator in UIAbstractTableView not displaying when sorted. --- .../eepp/ui/abstract/uiabstracttableview.hpp | 11 +++ src/eepp/ui/abstract/uiabstracttableview.cpp | 86 ++++++++++++++----- src/eepp/ui/uidropdown.cpp | 46 ++++++---- 3 files changed, 104 insertions(+), 39 deletions(-) diff --git a/include/eepp/ui/abstract/uiabstracttableview.hpp b/include/eepp/ui/abstract/uiabstracttableview.hpp index 24a166739..035b34f16 100644 --- a/include/eepp/ui/abstract/uiabstracttableview.hpp +++ b/include/eepp/ui/abstract/uiabstracttableview.hpp @@ -152,6 +152,10 @@ class EE_API UIAbstractTableView : public UIAbstractView { void setAutoColumnsWidth( bool autoColumnsWidth ); + /** Sorts @p colIndex through the model and reflects it in the header indicator, as if the user + * had clicked that column header. */ + virtual void sortByColumn( const size_t& colIndex, const SortOrder& sortOrder ); + const size_t& getMainColumn() const; /** The main column is the column that should be prioritized to occupy as much space as @@ -246,6 +250,9 @@ class EE_API UIAbstractTableView : public UIAbstractView { bool mUpdatingColumnsForScrollbars{ false }; bool mAutoExpandedColumnUsesVerticalScroll{ false }; std::string mPendingSerializedColumnWidths; + // Last sort state drawn in the header, so the indicator is only touched when it changes. + int mSortIndicatorColumn{ -1 }; + SortOrder mSortIndicatorOrder{ SortOrder::None }; virtual ~UIAbstractTableView(); @@ -296,6 +303,10 @@ class EE_API UIAbstractTableView : public UIAbstractView { virtual void onSortColumn( const size_t& colIndex ); + /** Draws the sort indicator on @p colIndex, clearing any indicator left on another column. + * Pass SortOrder::None to clear the indicator entirely. */ + void applySortIndicator( const size_t& colIndex, const SortOrder& sortOrder ); + virtual Uint32 onTextInput( const TextInputEvent& event ); virtual Uint32 onKeyDown( const KeyEvent& event ); diff --git a/src/eepp/ui/abstract/uiabstracttableview.cpp b/src/eepp/ui/abstract/uiabstracttableview.cpp index 2fdebf841..cdc5a2ce3 100644 --- a/src/eepp/ui/abstract/uiabstracttableview.cpp +++ b/src/eepp/ui/abstract/uiabstracttableview.cpp @@ -440,6 +440,17 @@ void UIAbstractTableView::createOrUpdateColumns( bool resetColumnData ) { mHeader->setVisible( visible ); updateColumnsWidth(); + + // Reflect the model's sort state in the header. The indicator is otherwise only produced by + // the header-click path, which leaves a programmatic sort (and the initial sort order of a + // freshly attached model) showing no indicator at all. + if ( model->isSortable() ) { + int keyColumn = model->keyColumn(); + // A model with no key column is unsorted, and its reported sort order is meaningless. + SortOrder sortOrder = keyColumn < 0 ? SortOrder::None : model->sortOrder(); + if ( keyColumn != mSortIndicatorColumn || sortOrder != mSortIndicatorOrder ) + applySortIndicator( keyColumn < 0 ? 0 : static_cast( keyColumn ), sortOrder ); + } } Float UIAbstractTableView::getHeaderHeight() const { @@ -1184,34 +1195,67 @@ void UIAbstractTableView::onRowCreated( UITableRow* row ) { sendEvent( &rowEvent ); } +void UIAbstractTableView::applySortIndicator( const size_t& colIndex, const SortOrder& sortOrder ) { + // Clear any indicator left on another column. + for ( size_t i = 0; i < mColumn.size(); ++i ) { + if ( i == colIndex || !mColumn[i].widget ) + continue; + UIImage* other = mColumn[i].widget->getExtraInnerWidget()->asType(); + if ( !other ) + continue; + other->setForegroundFillEnabled( false ); + other->setDrawable( DrawablePtr{} ); + } + + if ( sortOrder == SortOrder::None || colIndex >= mColumn.size() || !mColumn[colIndex].widget ) { + mSortIndicatorColumn = -1; + mSortIndicatorOrder = SortOrder::None; + return; + } + + UIPushButton* button = mColumn[colIndex].widget; + UIImage* image = button->getExtraInnerWidget()->asType(); + if ( !image ) { + mSortIndicatorColumn = -1; + mSortIndicatorOrder = SortOrder::None; + return; + } + + mSortIndicatorColumn = static_cast( colIndex ); + mSortIndicatorOrder = sortOrder; + + std::string tag = button->getElementTag() + "::arrow"; + image->setElementTag( sortOrder == SortOrder::Ascending ? tag + "-up" : tag + "-down" ); + image->setForegroundFillEnabled( true ); + image->reloadStyle(); + if ( image->getForeground() ) + image->getForeground()->setAlpha( 255 ); + if ( image->getForeground() == nullptr ) { + DrawablePtr icon = mUISceneNode->findIconDrawable( + sortOrder == SortOrder::Ascending ? "arrow-down" : "arrow-up", mSortIconSize ); + if ( icon ) + image->setDrawable( std::move( icon ) ); + } +} + +void UIAbstractTableView::sortByColumn( const size_t& colIndex, const SortOrder& sortOrder ) { + Model* model = getModel(); + if ( !model || !model->isSortable() || !model->isColumnSortable( colIndex ) ) + return; + + // Sorting notifies the views, which refresh the header and pick the indicator up from the + // model's new state. + model->sort( colIndex, sortOrder ); +} + void UIAbstractTableView::onSortColumn( const size_t& colIndex ) { Model* model = getModel(); if ( !model ) return; if ( model->isSortable() && model->isColumnSortable( colIndex ) ) { - if ( -1 != model->keyColumn() && (Int64)colIndex != model->keyColumn() && - columnData( model->keyColumn() ).widget ) { - UIImage* image = - columnData( model->keyColumn() ).widget->getExtraInnerWidget()->asType(); - image->setForegroundFillEnabled( false ); - image->setDrawable( DrawablePtr{} ); - } SortOrder sortOrder = model->sortOrder() == SortOrder::Ascending ? SortOrder::Descending : SortOrder::Ascending; - UIPushButton* button = columnData( colIndex ).widget; - UIImage* image = button->getExtraInnerWidget()->asType(); - std::string tag = button->getElementTag() + "::arrow"; - image->setElementTag( sortOrder == SortOrder::Ascending ? tag + "-up" : tag + "-down" ); - image->setForegroundFillEnabled( true ); - image->reloadStyle(); - if ( image->getForeground() ) - image->getForeground()->setAlpha( 255 ); - if ( image && image->getForeground() == nullptr ) { - DrawablePtr icon = mUISceneNode->findIconDrawable( - sortOrder == SortOrder::Ascending ? "arrow-down" : "arrow-up", mSortIconSize ); - if ( icon ) - image->setDrawable( std::move( icon ) ); - } + applySortIndicator( colIndex, sortOrder ); model->sort( colIndex, sortOrder ); } } diff --git a/src/eepp/ui/uidropdown.cpp b/src/eepp/ui/uidropdown.cpp index fa3e3af16..6e276a7cb 100644 --- a/src/eepp/ui/uidropdown.cpp +++ b/src/eepp/ui/uidropdown.cpp @@ -139,28 +139,38 @@ void UIDropDown::alignPopUp( UIWidget* widget ) { bool center = mStyleConfig.menuWidthRule == MenuWidthMode::ContentsCentered || mStyleConfig.menuWidthRule == MenuWidthMode::ExpandIfNeededCentered; - Float width = widget->getSize().getWidth(); - Float offsetX = center ? eefloor( ( getSize().getWidth() - width ) * 0.5f ) : 0; + // The placement is decided in screen pixels, where the field, the popup and the scene bounds + // are directly comparable. Deriving it from the node's own dp position previously mixed + // coordinate spaces (the candidate point was expressed in the parent's space but converted + // through the field's own nodeToWorld, shifting the test rectangle by the field's offset), so + // the "fits below" check failed for any field away from its parent's origin and the popup was + // flipped above the field even when there was no room there. + const Rectf field( getScreenRect() ); + const Sizef popUpSize( widget->getPixelsSize() ); + const Rectf sceneBounds( getUISceneNode()->getWorldBounds() ); - Vector2f pos( mDpPos.x + offsetX, mDpPos.y + getSize().getHeight() ); - Vector2f posCpy( pos ); - nodeToWorld( posCpy ); + Float x = center ? field.Left + eefloor( ( field.getWidth() - popUpSize.getWidth() ) * 0.5f ) + : field.Left; - if ( !getUISceneNode()->getWorldBounds().contains( Rectf( posCpy, widget->getSize() ) ) ) { - pos = Vector2f( mDpPos.x + offsetX, mDpPos.y - widget->getSize().getHeight() ); + // Prefer below the field, fall back to above it, and only then clamp: the list is never placed + // partially off screen when the scene has room for it on either side. + Float y = field.Bottom; + if ( y + popUpSize.getHeight() > sceneBounds.Bottom ) { + Float above = field.Top - popUpSize.getHeight(); + y = above >= sceneBounds.Top + ? above + : eemax( sceneBounds.Top, sceneBounds.Bottom - popUpSize.getHeight() ); } - if ( mStyleConfig.PopUpToRoot ) { - getParent()->nodeToWorld( pos ); - pos = PixelDensity::pxToDp( pos ); - } else { - Node* parentNode = getParent(); - Node* rp = getWindowContainer(); - while ( rp != parentNode ) { - pos += parentNode->getPosition(); - parentNode = parentNode->getParent(); - } - } + // Keep the popup inside the scene horizontally as well; a list wider than its field used to + // run off the right edge. + x = eeclamp( x, sceneBounds.Left, + eemax( sceneBounds.Left, sceneBounds.Right - popUpSize.getWidth() ) ); + + // World coordinates are pixels and worldToNode already converts back to dp, which is what + // setPosition expects; applying the density a second time would shift the popup. + Vector2f pos( x, y ); + widget->getParent()->worldToNode( pos ); widget->setPosition( pos ); show();