[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