[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: [Differential] [Updated] D3210: make scrollbar size configurable
From: "hpereiradacosta (Hugo Pereira Da Costa)" <noreply () phabricator ! kde ! org>
Date: 2016-10-31 16:14:15
Message-ID: 20161031161414.34695.28530.3BC74DB9 () phabricator ! kde ! org
[Download RAW message or body]
hpereiradacosta added a comment.
Hello Marco,
Thanks for the patch.
1/ On the implementation side: I have tried so far to keep all the magic numbers \
for breeze (and oxygen) in Breeze::Metrics, because it makes maintainability much \
easier. I think it should stays so, and not become split between Metrics and Helper. \
In the current implementation I see two ways to do this:
- the easy way: move the enums back to Breeze::Metrics, using e.g.
ScrollBar_Extend_Tiny, ScrollBar_SliderWidth_Tiny, ScrollBar_MinSliderHeight_Tiny \
(and the same widht _Default and with _Large) and let Helper (as it is now), switch \
between one set and the other (with no more magic nubers
- the not so easy way: make metrics a real class and move all the new Helper code \
there.
Either way is fine with me, for the moment.
2/ on the actual feature side:
- Large: I really don't think we need this. It looks oversized with respect to the \
other breeze elements, and nobody (as far as I know) never asked for larger \
scrollbars. I think "small" and "medium" should be enough. And I would rename them to \
"default" and large. (if indeed small is the default)
- small: I think scrollBar_MinSliderHeight should be increased to 10 (or 12). \
Hitting the handle when the scrollbar has its minimum size at 6, becomes very \
difficult (try open a large file in e.g. kate or kwrite). This makes the handle \
non-circular, but I think this is acceptable.
Still on small: I think there is something not elegant with the size of the groove \
and the size of the arrows on both sides of the scrollbar, when visible. arrows \
appear too large. Should we make them smaller ?
Disable them by default ?
making them smaller has the inconvenient that it will become inconsistent with the \
other arrows in the rest of the style.
On the other hand, using 8 instead of 6 for the groove width, make the two more \
balanced. (but then it makes again the scrollbar larger than the progressbars).
Finally: on the need for an option (and this is true with the other commit).
Do we really need one ? Will we add an option every time we change something in the \
style's appearance from now on ? I can see how this will turn breeze into qtcurve \
in the future, and make it unmaintainable.
Also I thought the original idea was to make breeze consistent by design, so that \
you do not need options. In fact, options would be a nuisance: they would either \
break consistency, or make it appear as if we - or rather the designers- are not sure \
about their designs.
I think Nuno's back in the oxygen days, also agreed with that.
In other words, I do not think having an option for whether scrollbar grove is \
shown on mouse-over or not, and another one on the width of the scrollbar, enters the \
category of "powerful when needed", so as to justify an option (well two options).
I would _really_ like to have the oppinion of VDG on that, but IMO, if we are sure \
about ourselves on this new scrollbar design we should just make these two defaults \
the only available options and stick to it.
Sorry for the long posting
REPOSITORY
rBREEZE Breeze
REVISION DETAIL
https://phabricator.kde.org/D3210
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: mart, #plasma, #vdg, hpereiradacosta
Cc: plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
[Attachment #3 (text/html)]
<table><tr><td style="">hpereiradacosta added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: \
right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: \
#F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: \
inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D3210" rel="noreferrer">View \
Revision</a></tr></table><br /><div><div><p>Hello Marco,<br /> Thanks for the patch. \
<br /> 1/ On the implementation side: I have tried so far to keep all the magic \
numbers for breeze (and oxygen) in Breeze::Metrics, because it makes maintainability \
much easier. I think it should stays so, and not become split between Metrics and \
Helper. In the current implementation I see two ways to do this:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">the easy way: move the enums back to Breeze::Metrics, \
using e.g.</li> </ul>
<p>ScrollBar_Extend_Tiny, ScrollBar_SliderWidth_Tiny, ScrollBar_MinSliderHeight_Tiny \
(and the same widht _Default and with _Large) and let Helper (as it is now), switch \
between one set and the other (with no more magic nubers</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">the not so easy way: make metrics a real class and \
move all the new Helper code there.</li> </ul>
<p>Either way is fine with me, for the moment.</p>
<p>2/ on the actual feature side:</p>
<ul class="remarkup-list">
<li class="remarkup-list-item">Large: I really don't think we need this. It \
looks oversized with respect to the other breeze elements, and nobody (as far as I \
know) never asked for larger scrollbars. I think "small" and \
"medium" should be enough. And I would rename them to "default" \
and large. (if indeed small is the default)</li> </ul>
<ul class="remarkup-list">
<li class="remarkup-list-item">small: I think scrollBar_MinSliderHeight should be \
increased to 10 (or 12). Hitting the handle when the scrollbar has its minimum size \
at 6, becomes very difficult (try open a large file in e.g. kate or kwrite). This \
makes the handle non-circular, but I think this is acceptable.</li> </ul>
<p>Still on small: I think there is something not elegant with the size of the groove \
and the size of the arrows on both sides of the scrollbar, when visible. arrows \
appear too large. <br /> Should we make them smaller ? <br />
Disable them by default ? <br />
making them smaller has the inconvenient that it will become inconsistent with the \
other arrows in the rest of the style.</p>
<p>On the other hand, using 8 instead of 6 for the groove width, make the two more \
balanced. (but then it makes again the scrollbar larger than the progressbars).</p>
<p>Finally: on the need for an option (and this is true with the other commit). <br \
/> Do we really need one ? Will we add an option every time we change something in \
the style's appearance from now on ? <br /> I can see how this will turn breeze \
into qtcurve in the future, and make it unmaintainable.</p>
<p>Also I thought the original idea was to make breeze consistent by design, so that \
you do not need options. In fact, options would be a nuisance: they would either \
break consistency, or make it appear as if we - or rather the designers- are not sure \
about their designs.</p>
<p>I think Nuno's back in the oxygen days, also agreed with that.</p>
<p>In other words, I do not think having an option for whether scrollbar grove is \
shown on mouse-over or not, and another one on the width of the scrollbar, enters the \
category of "powerful when needed", so as to justify an option (well two \
options).</p>
<p>I would _really_ like to have the oppinion of VDG on that, but IMO, if we are sure \
about ourselves on this new scrollbar design we should just make these two defaults \
the only available options and stick to it.</p>
<p>Sorry for the long posting</p></div></div><br \
/><div><strong>REPOSITORY</strong><div><div>rBREEZE Breeze</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D3210" \
rel="noreferrer">https://phabricator.kde.org/D3210</a></div></div><br \
/><div><strong>EMAIL PREFERENCES</strong><div><a \
href="https://phabricator.kde.org/settings/panel/emailpreferences/" \
rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br \
/><div><strong>To: </strong>mart, Plasma, VDG, hpereiradacosta<br /><strong>Cc: \
</strong>plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br \
/></div>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic