View Issue Details

IDProjectCategoryView StatusLast Update
0016890ScribusUser Interfacepublic2023-01-17 18:28
Reporterpmjdebruijn Assigned To 
PrioritynormalSeverityminorReproducibilityalways
Status newResolutionopen 
Product Version1.5.9.svn 
Summary0016890: Colors and Fills dialog too narrow
DescriptionCurrently many color names get ellipsized in a Colors and Fills dialog.

With this patch, the listbox will be wider, thus able to show most colors names without ellipsizing.
TagsNo tags attached.
PatchYes

Activities

pmjdebruijn

2023-01-10 17:23

reporter  

colorsandfills.patch (435 bytes)   
Index: a/scribus/ui/colorsandfillsbase.ui
===================================================================
--- a/scribus/ui/colorsandfillsbase.ui	(revision 25243)
+++ a/scribus/ui/colorsandfillsbase.ui	(working copy)
@@ -6,8 +6,8 @@
    <rect>
     <x>0</x>
     <y>0</y>
-    <width>412</width>
-    <height>412</height>
+    <width>532</width>
+    <height>427</height>
    </rect>
   </property>
   <property name="windowTitle">
colorsandfills.patch (435 bytes)   

pmjdebruijn

2023-01-12 17:09

reporter   ~0049887

Here's a screenshot illustrating the "new" wider dialog, where most color names aren't ellipsized:
scribus_colors_and_fills.png (85,960 bytes)   
scribus_colors_and_fills.png (85,960 bytes)   

jghali

2023-01-12 23:34

administrator   ~0049898

Not a fan of this patch. The length of color names depends on current system font and theme. On Windows, with current dialog width, just a few colors names are ellipsized in the case of Wikipedia RAL Classic swatch. With your patch the dialog appears much too wide, with lots of useless blank space.

pmjdebruijn

2023-01-13 16:11

reporter   ~0049901

@jghali while I understand your point, I'm not sure whether that's the right perspective...

[1] If the list is too wide, and there's blank space, that has little to no practical disadvantage
[2] if the list is too narrow, and entries are ellipsized, colornames can't be read without resizing the window, forcing users to act on something, that really shouldn't require any action in 99% of cases.

So I'd argue that [1] is much less worse than [2].

The width I chose is a compromise already, and does not remove ellipsization entirely, going over all the included swatches there's still longer color names that will still be ellipsized, but this now happens much less than without my patch.

PeterBenedek

2023-01-13 16:50

developer   ~0049903

" So I'd argue that [1] is much less worse than [2]."

I agree.

jghali

2023-01-13 17:36

administrator   ~0049904

Last edited: 2023-01-13 17:37

For me in the case of [1], my first reaction will be to reduce the dialog size, especially as with this patch it gets almost two times as wide as necessary. So for me [1] is as bad as [2].

jghali

2023-01-13 17:43

administrator   ~0049905

Imo a solution could be to use QFontMetrics functionnalities. The idea would be to guarantee that the tree view can display a certain minimum number of character by default. QFontMetrics allows us to retrieve an average char width (by using averageCharWidth()) for a quick estimation or text size by using boundingRect() or size() for a more precise estimation.

jghali

2023-01-14 00:26

administrator   ~0049906

Here is my attempt at using QFontMetrics. Some adjustments related to screen pixel ratio may still be needed. I'll have to test on my other laptop tomorrow.
16890_colorsandfills.patch (1,842 bytes)   
Index: scribus/ui/colorsandfills.cpp
===================================================================
--- scribus/ui/colorsandfills.cpp	(revision 25247)
+++ scribus/ui/colorsandfills.cpp	(working copy)
@@ -32,10 +32,12 @@
 #include <QDomDocument>
 #include <QEventLoop>
 #include <QFileDialog>
+#include <QFontMetrics>
 #include <QImageReader>
 #include <QMenu>
 #include <QMessageBox>
 #include <QScopedPointer>
+#include <QScreen>
 
 #include "cmykfw.h"
 #include "colorlistbox.h"
@@ -148,8 +150,28 @@
 	dataTree->expandItem(patternItems);
 	dataTree->setSelectionMode(QAbstractItemView::ExtendedSelection);
 	dataTree->setCurrentItem(dataTree->topLevelItem(0));
+	dataTree->adjustSize(); // In order to get proper size
 	itemSelected(dataTree->currentItem());
 
+	QFontMetrics fontMetrics(dataTree->font());
+
+	int maxAdvance = 0;
+	for (QString colorName : m_colorList.keys())
+		maxAdvance = std::max(maxAdvance, fontMetrics.horizontalAdvance(colorName));
+	int maxTreeWidth = dataTree->screen()->size().width() * 0.5;
+	int idealTreeWidth = maxAdvance + 75;
+	idealTreeWidth += fontMetrics.horizontalAdvance(colorItems->child(0)->text(0)) / 2.0;
+	idealTreeWidth += dataTree->verticalScrollBar()->height();
+	idealTreeWidth = std::min(idealTreeWidth, maxTreeWidth);
+
+	QSize dataTreeSize = dataTree->size();
+	if (dataTreeSize.width() < idealTreeWidth)
+	{
+		int diffWidth = idealTreeWidth - dataTreeSize.width();
+		QSize diaSize = this->size();
+		this->resize(diaSize.width() + diffWidth, diaSize.height());
+	}
+
 	connect(dataTree, SIGNAL(itemClicked(QTreeWidgetItem*, int)), this, SLOT(itemSelected(QTreeWidgetItem*)));
 	connect(dataTree, SIGNAL(itemDoubleClicked(QTreeWidgetItem*, int)), this, SLOT(selEditColor(QTreeWidgetItem*)));
 	connect(dataTree, SIGNAL(itemSelectionChanged()), this, SLOT(itemSelectionChanged()));
16890_colorsandfills.patch (1,842 bytes)   

ale

2023-01-14 08:31

manager   ~0049909

i wonder if switching the panes would allow to have a list with a variable width...
scribus_colors_and_fills-2.png (91,912 bytes)   
scribus_colors_and_fills-2.png (91,912 bytes)   

pmjdebruijn

2023-01-14 11:39

reporter   ~0049912

@jghali, thanks for the patch, but as-is there's no point, two reasons:

[1] It resizes the Tree, not the window, it's not good to have the buttons change size.
[2] It resizes the Tree, when the window is opened, based on the current contents of the tree, which means it will be as tight as it can, which by default is way to tight, meaning, there are loads of colornames (if added) that will still be elipsized.

Having to close/re-open to window for it to resize, isn't faster/easier than just resizing manually, which is what this ticket was all about in the first place.

What would be much more useful, is to determine the window size on a static string, for example something along these lines:
fontMetrics.horizontalAdvance("HCL 000 00 00 Bright Orange")

That would still account for different fonts, and leave rooms for most names to not be ellipsized, with a few rare exceptions.

jghali

2023-01-14 17:37

administrator   ~0049918

I'm wondering whether we're in fact wrong about this issue and if we should just make this dialog remember its size like palettes or file dialog. One thing I've noticed tho is that the dialog resize itself when loading new palettes and that's bad, that will require additional fixes.

pmjdebruijn

2023-01-15 12:12

reporter   ~0049928

The dialog remembering it's size, doesn't fix the real issue. The real issue, is that it's way too narrow by default, in which case the user would still commonly need to make it wider. This should rarely if ever be needed.

jghali

2023-01-15 14:38

administrator   ~0049931

Last edited: 2023-01-15 14:44

>> The real issue, is that it's way too narrow by default

Sorry, but that's not the case on Windows. So I don't see why I would make a change which would make the dialog way too big for 80% of our users. The dialog remembering its size would be a compromise solution.

jghali

2023-01-15 14:46

administrator   ~0049932

>> One thing I've noticed tho is that the dialog resize itself when loading new palettes and that's bad, that will require additional fixes.

Unfortunately it looks like that one will be tricky to fix.

cbradney

2023-01-15 18:28

administrator   ~0049933

I agree that something needs to be done to this window. In 1.7.x, we should rewrite it.
For 1.6.0, we need to get it to a reasonable state. Its not acceptable how it is now.

jghali

2023-01-17 00:08

administrator   ~0049941

Last edited: 2023-01-17 00:11

Arghhh! It seems QFontMetrics is not giving expected results on some screens. Probably something related to logicalDpi() / physicalDpi() / devicePixelRatio().

pmjdebruijn

2023-01-17 18:28

reporter   ~0049942

Maybe this should just be an #ifdef !_WIN32 then resize(532, 427) for the time being?

Issue History

Date Modified Username Field Change
2023-01-10 17:23 pmjdebruijn New Issue
2023-01-10 17:23 pmjdebruijn File Added: colorsandfills.patch
2023-01-12 17:09 pmjdebruijn Note Added: 0049887
2023-01-12 17:09 pmjdebruijn File Added: scribus_colors_and_fills.png
2023-01-12 23:34 jghali Note Added: 0049898
2023-01-13 16:11 pmjdebruijn Note Added: 0049901
2023-01-13 16:50 PeterBenedek Note Added: 0049903
2023-01-13 17:36 jghali Note Added: 0049904
2023-01-13 17:37 jghali Note Edited: 0049904
2023-01-13 17:43 jghali Note Added: 0049905
2023-01-14 00:26 jghali Note Added: 0049906
2023-01-14 00:26 jghali File Added: 16890_colorsandfills.patch
2023-01-14 08:31 ale Note Added: 0049909
2023-01-14 08:31 ale File Added: scribus_colors_and_fills-2.png
2023-01-14 11:39 pmjdebruijn Note Added: 0049912
2023-01-14 17:37 jghali Note Added: 0049918
2023-01-15 12:12 pmjdebruijn Note Added: 0049928
2023-01-15 14:38 jghali Note Added: 0049931
2023-01-15 14:44 jghali Note Edited: 0049931
2023-01-15 14:46 jghali Note Added: 0049932
2023-01-15 18:28 cbradney Note Added: 0049933
2023-01-17 00:08 jghali Note Added: 0049941
2023-01-17 00:11 jghali Note Edited: 0049941
2023-01-17 18:28 pmjdebruijn Note Added: 0049942