[v2] qcam: Decrease minimum width of selector dialog
diff mbox series

Message ID 20240905141045.26165-1-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • [v2] qcam: Decrease minimum width of selector dialog
Related show

Commit Message

Laurent Pinchart Sept. 5, 2024, 2:10 p.m. UTC 
From: Luca Weiss <luca@z3ntu.xyz>

On phone screens the default width is too wide, so the OK button cannot
be clicked.

Fix this by decreasing the minimum size of the dialog so it fits nicely.

Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
I found this patch in a branch in my tree. The v2 incorporates the
change I proposed in the review of v1. The discussion died out after
that, so I thought posting a new version could help getting it merged.

I've dropped the R-b tags as the implementation has changed.
---
Changes since v1:

- Make the minimum width depend on the screen geometry
- Only decrease the width
---
 src/apps/qcam/cam_select_dialog.cpp | 10 ++++++++++
 1 file changed, 10 insertions(+)


base-commit: f75b8dd26feaca86701704390dea18c71e2f0350

Comments

Milan Zamazal Sept. 5, 2024, 3:29 p.m. UTC | #1 
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> From: Luca Weiss <luca@z3ntu.xyz>
>
> On phone screens the default width is too wide, so the OK button cannot
> be clicked.
>
> Fix this by decreasing the minimum size of the dialog so it fits nicely.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> I found this patch in a branch in my tree. The v2 incorporates the
> change I proposed in the review of v1. The discussion died out after
> that, so I thought posting a new version could help getting it merged.
>
> I've dropped the R-b tags as the implementation has changed.

I don't know the history but the change looks reasonable and good to me.
Struggling with app geometries myself on my Linux phone, I welcome any
such change. :-)

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
> Changes since v1:
>
> - Make the minimum width depend on the screen geometry
> - Only decrease the width
> ---
>  src/apps/qcam/cam_select_dialog.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
> index c51f59745e48..6b6d0713cc3d 100644
> --- a/src/apps/qcam/cam_select_dialog.cpp
> +++ b/src/apps/qcam/cam_select_dialog.cpp
> @@ -15,7 +15,9 @@
>  #include <QComboBox>
>  #include <QDialogButtonBox>
>  #include <QFormLayout>
> +#include <QGuiApplication>
>  #include <QLabel>
> +#include <QScreen>
>  #include <QString>
>  
>  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> @@ -53,6 +55,14 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
>  	layout->addRow("Location:", cameraLocation_);
>  	layout->addRow("Model:", cameraModel_);
>  	layout->addWidget(buttonBox);
> +
> +	/*
> +	 * Decrease the minimum width of dialog to fit on narrow screens, with a
> +	 * 20 pixels margin.
> +	 */
> +	QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
> +	if (screenGeometry.width() < minimumWidth())
> +		setMinimumWidth(screenGeometry.width() - 20);
>  }
>  
>  CameraSelectorDialog::~CameraSelectorDialog() = default;
>
> base-commit: f75b8dd26feaca86701704390dea18c71e2f0350
Kieran Bingham Sept. 5, 2024, 3:48 p.m. UTC | #2 
Quoting Laurent Pinchart (2024-09-05 15:10:45)
> From: Luca Weiss <luca@z3ntu.xyz>
> 
> On phone screens the default width is too wide, so the OK button cannot
> be clicked.
> 
> Fix this by decreasing the minimum size of the dialog so it fits nicely.
> 
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> I found this patch in a branch in my tree. The v2 incorporates the
> change I proposed in the review of v1. The discussion died out after
> that, so I thought posting a new version could help getting it merged.

I have this stuck in some old branches of mine too so I'm happy to see
it get merged too ;-)


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> I've dropped the R-b tags as the implementation has changed.
> ---
> Changes since v1:
> 
> - Make the minimum width depend on the screen geometry
> - Only decrease the width
> ---
>  src/apps/qcam/cam_select_dialog.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
> index c51f59745e48..6b6d0713cc3d 100644
> --- a/src/apps/qcam/cam_select_dialog.cpp
> +++ b/src/apps/qcam/cam_select_dialog.cpp
> @@ -15,7 +15,9 @@
>  #include <QComboBox>
>  #include <QDialogButtonBox>
>  #include <QFormLayout>
> +#include <QGuiApplication>
>  #include <QLabel>
> +#include <QScreen>
>  #include <QString>
>  
>  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> @@ -53,6 +55,14 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
>         layout->addRow("Location:", cameraLocation_);
>         layout->addRow("Model:", cameraModel_);
>         layout->addWidget(buttonBox);
> +
> +       /*
> +        * Decrease the minimum width of dialog to fit on narrow screens, with a
> +        * 20 pixels margin.
> +        */
> +       QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
> +       if (screenGeometry.width() < minimumWidth())
> +               setMinimumWidth(screenGeometry.width() - 20);
>  }
>  
>  CameraSelectorDialog::~CameraSelectorDialog() = default;
> 
> base-commit: f75b8dd26feaca86701704390dea18c71e2f0350
> -- 
> Regards,
> 
> Laurent Pinchart
>
Laurent Pinchart Sept. 5, 2024, 3:53 p.m. UTC | #3 
On Thu, Sep 05, 2024 at 04:48:30PM +0100, Kieran Bingham wrote:
> Quoting Laurent Pinchart (2024-09-05 15:10:45)
> > From: Luca Weiss <luca@z3ntu.xyz>
> > 
> > On phone screens the default width is too wide, so the OK button cannot
> > be clicked.
> > 
> > Fix this by decreasing the minimum size of the dialog so it fits nicely.
> > 
> > Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > I found this patch in a branch in my tree. The v2 incorporates the
> > change I proposed in the review of v1. The discussion died out after
> > that, so I thought posting a new version could help getting it merged.
> 
> I have this stuck in some old branches of mine too so I'm happy to see
> it get merged too ;-)

For what it's worth, I tested this with a hack by hardcoding the minimum
to a smaller value than the default size of the dialog on my system, and
it then gets displayed with a smaller size.

> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > 
> > I've dropped the R-b tags as the implementation has changed.
> > ---
> > Changes since v1:
> > 
> > - Make the minimum width depend on the screen geometry
> > - Only decrease the width
> > ---
> >  src/apps/qcam/cam_select_dialog.cpp | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
> > index c51f59745e48..6b6d0713cc3d 100644
> > --- a/src/apps/qcam/cam_select_dialog.cpp
> > +++ b/src/apps/qcam/cam_select_dialog.cpp
> > @@ -15,7 +15,9 @@
> >  #include <QComboBox>
> >  #include <QDialogButtonBox>
> >  #include <QFormLayout>
> > +#include <QGuiApplication>
> >  #include <QLabel>
> > +#include <QScreen>
> >  #include <QString>
> >  
> >  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
> > @@ -53,6 +55,14 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
> >         layout->addRow("Location:", cameraLocation_);
> >         layout->addRow("Model:", cameraModel_);
> >         layout->addWidget(buttonBox);
> > +
> > +       /*
> > +        * Decrease the minimum width of dialog to fit on narrow screens, with a
> > +        * 20 pixels margin.
> > +        */
> > +       QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
> > +       if (screenGeometry.width() < minimumWidth())
> > +               setMinimumWidth(screenGeometry.width() - 20);
> >  }
> >  
> >  CameraSelectorDialog::~CameraSelectorDialog() = default;
> > 
> > base-commit: f75b8dd26feaca86701704390dea18c71e2f0350
Luca Weiss Sept. 5, 2024, 5:36 p.m. UTC | #4 
Thanks for picking this up again!
(currently on my phone so no clue what the quoting will look like)

Regards
Luca

Laurent Pinchart <laurent.pinchart@ideasonboard.com> schreef op 5 september 2024 16:10:45 CEST:
>From: Luca Weiss <luca@z3ntu.xyz>
>
>On phone screens the default width is too wide, so the OK button cannot
>be clicked.
>
>Fix this by decreasing the minimum size of the dialog so it fits nicely.
>
>Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
>Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>---
>I found this patch in a branch in my tree. The v2 incorporates the
>change I proposed in the review of v1. The discussion died out after
>that, so I thought posting a new version could help getting it merged.
>
>I've dropped the R-b tags as the implementation has changed.
>---
>Changes since v1:
>
>- Make the minimum width depend on the screen geometry
>- Only decrease the width
>---
> src/apps/qcam/cam_select_dialog.cpp | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
>index c51f59745e48..6b6d0713cc3d 100644
>--- a/src/apps/qcam/cam_select_dialog.cpp
>+++ b/src/apps/qcam/cam_select_dialog.cpp
>@@ -15,7 +15,9 @@
> #include <QComboBox>
> #include <QDialogButtonBox>
> #include <QFormLayout>
>+#include <QGuiApplication>
> #include <QLabel>
>+#include <QScreen>
> #include <QString>
> 
> CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
>@@ -53,6 +55,14 @@ CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
> 	layout->addRow("Location:", cameraLocation_);
> 	layout->addRow("Model:", cameraModel_);
> 	layout->addWidget(buttonBox);
>+
>+	/*
>+	 * Decrease the minimum width of dialog to fit on narrow screens, with a
>+	 * 20 pixels margin.
>+	 */
>+	QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
>+	if (screenGeometry.width() < minimumWidth())
>+		setMinimumWidth(screenGeometry.width() - 20);
> }
> 
> CameraSelectorDialog::~CameraSelectorDialog() = default;
>
>base-commit: f75b8dd26feaca86701704390dea18c71e2f0350

Patch
diff mbox series 

diff --git a/src/apps/qcam/cam_select_dialog.cpp b/src/apps/qcam/cam_select_dialog.cpp
index c51f59745e48..6b6d0713cc3d 100644
--- a/src/apps/qcam/cam_select_dialog.cpp
+++ b/src/apps/qcam/cam_select_dialog.cpp
@@ -15,7 +15,9 @@ 
 #include <QComboBox>
 #include <QDialogButtonBox>
 #include <QFormLayout>
+#include <QGuiApplication>
 #include <QLabel>
+#include <QScreen>
 #include <QString>
 
 CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManager,
@@ -53,6 +55,14 @@  CameraSelectorDialog::CameraSelectorDialog(libcamera::CameraManager *cameraManag
 	layout->addRow("Location:", cameraLocation_);
 	layout->addRow("Model:", cameraModel_);
 	layout->addWidget(buttonBox);
+
+	/*
+	 * Decrease the minimum width of dialog to fit on narrow screens, with a
+	 * 20 pixels margin.
+	 */
+	QRect screenGeometry = qGuiApp->primaryScreen()->availableGeometry();
+	if (screenGeometry.width() < minimumWidth())
+		setMinimumWidth(screenGeometry.width() - 20);
 }
 
 CameraSelectorDialog::~CameraSelectorDialog() = default;