Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix pointcloud properties dialog minimum size #55976

Merged
merged 8 commits into from
Feb 8, 2024
Merged

Fix pointcloud properties dialog minimum size #55976

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
556a119
reduce minimum height for point cloud renderer widgets
uclaros Jan 24, 2024
913b821
Add a QgsStackedWidget class that only takes into account its current…
uclaros Jan 24, 2024
f6e704f
Consider all pages sizes by default
uclaros Feb 1, 2024
9015c7f
Rename enum value
uclaros Feb 1, 2024
6d56b11
doh'string
uclaros Feb 1, 2024
6218dda
spellok
uclaros Feb 7, 2024
b060659
spellok juggling
uclaros Feb 7, 2024
0b6ca1c
spellok juggling
uclaros Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/gui/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ set(QGIS_GUI_SRCS
qgssublayersdialog.cpp
qgssubstitutionlistwidget.cpp
qgssqlcomposerdialog.cpp
qgsstackedwidget.cpp
qgsstatusbar.cpp
qgssymbolbutton.cpp
qgssymbollayerselectionwidget.cpp
Expand Down Expand Up @@ -1000,6 +1001,7 @@ set(QGIS_GUI_HDRS
qgssourceselectprovider.h
qgssourceselectproviderregistry.h
qgssqlcomposerdialog.h
qgsstackedwidget.h
qgsstatusbar.h
qgsstyleitemslistwidget.h
qgssublayersdialog.h
Expand Down
35 changes: 35 additions & 0 deletions src/gui/qgsstackedwidget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/***************************************************************************
qgsstackedwidget.cpp
--------------------
begin : January 2024
copyright : (C) 2024 by Stefanos Natsis
email : uclaros at gmail dot com
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#include "qgsstackedwidget.h"

#include <QStackedWidget>
#include <QSize>


QgsStackedWidget::QgsStackedWidget( QWidget *parent )
: QStackedWidget( parent )
{
}

QSize QgsStackedWidget::sizeHint() const
{
return currentWidget() ? currentWidget()->sizeHint() : QSize();
}

QSize QgsStackedWidget::minimumSizeHint() const
{
return currentWidget() ? currentWidget()->minimumSizeHint() : QSize();
}
53 changes: 53 additions & 0 deletions src/gui/qgsstackedwidget.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/***************************************************************************
qgsstackedwidget.h
------------------
begin : January 2024
copyright : (C) 2024 by Stefanos Natsis
email : uclaros at gmail dot com
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/

#ifndef QGSSTACKEDWIDGET_H
#define QGSSTACKEDWIDGET_H

#define SIP_NO_FILE
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's expose, there's potential value here for plugins

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we maybe backport this as is and then expose and expand on a separate PR?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with that


#include <QStackedWidget>
#include "qgis_gui.h"

class QSize;

/**
* \class QgsStackedWidget
* \ingroup gui
* \brief A QStackedWidget that can be shrunk to its current widget's size.
*
* A regular QStackedWidget can be shrunk down the size of its
* largest page widget. A QgsStackedWidget only takes the current
* page widget into account when resizing.
*
* \since QGIS 3.36
*/
class GUI_EXPORT QgsStackedWidget : public QStackedWidget
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice approach!

Can I suggest for future proofness that we:

  1. Add an enum:
enum class SizeMode {
    ConsiderAllPages, //!< The sizes of all pages are considered when calculating the stacked widget size
    CurrentPage, //!< Only the size of the current page is considered when calculating the stacked widget size
};
  1. Add getters/setters for SizeMode, default SizeMode to ConsiderAllPages (ie match QStackedWidget)
  2. Adapt the subclass sizeHint methods to consider the SizeMode

This way we could start moving everything over from QStackedWidget to QgsStackedWidget, and we'd have a natural place to add other tweaks and improvements in future. Much like what we do for QgsScrollArea. (There's also a test to force use of QgsScrollArea over QScrollArea, we could potentially add the same for QgsStackedWidget later)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess we could, I'm not sure about the potential gain (other than homogeneity). In most places we use a stacked widget it either holds similar widgets or it is used as a temporary container and the inactive page gets deleted.
With QgsScrollArea it was quite different, as it implemented the absolutely necessary functionality to avoid messing with widgets while scrolling.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's definitely value in this minimal height stacked widget elsewhere. There's lots of places where we have ugly empty space in dialogs because a stacked widget has forced a too large minimum height for the current page, and all the widgets under that get pushed down for no visible reason.

I'd still push for the enum setting added, because this is kind of our one change to do that and have the default behavior match QStackedWidget. If we need to add other tweaks to QgsStackedWidget in future then things get a bit messier as we'd need to avoid breaking the initial default behaviour.

{
Q_OBJECT

public:

/**
* Constructor for QgsStackedWidget.
*/
explicit QgsStackedWidget( QWidget *parent = nullptr );

QSize sizeHint() const override;
QSize minimumSizeHint() const override;

};

#endif // QGSSTACKEDWIDGET_H
8 changes: 7 additions & 1 deletion src/ui/3d/qgspointcloud3dsymbolwidget.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@
</widget>
</item>
<item row="2" column="0" colspan="3">
<widget class="QStackedWidget" name="mStackedWidget">
<widget class="QgsStackedWidget" name="mStackedWidget">
<property name="currentIndex">
<number>0</number>
</property>
Expand Down Expand Up @@ -637,6 +637,12 @@ enhancement</string>
<extends>QComboBox</extends>
<header>qgspointcloudattributecombobox.h</header>
</customwidget>
<customwidget>
<class>QgsStackedWidget</class>
<extends>QStackedWidget</extends>
<header>qgsstackedwidget.h</header>
<container>1</container>
</customwidget>
</customwidgets>
<tabstops>
<tabstop>mRenderingStyleComboBox</tabstop>
Expand Down
2 changes: 1 addition & 1 deletion src/ui/pointcloud/qgspointcloudclassifiedrendererwidgetbase.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<property name="minimumSize">
<size>
<width>0</width>
<height>250</height>
<height>100</height>
</size>
</property>
<property name="contextMenuPolicy">
Expand Down
2 changes: 1 addition & 1 deletion src/ui/qgscolorrampshaderwidgetbase.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
<property name="minimumSize">
<size>
<width>0</width>
<height>250</height>
<height>100</height>
</size>
</property>
<attribute name="headerMinimumSectionSize">
Expand Down
Loading