D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:854a62e57772: Remove stepsize 24 from DPI spinbox inside 
kcm fontconfig (authored by kurmikon, committed by ngraham).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29431?vs=82001&id=82672

REVISION DETAIL
  https://phabricator.kde.org/D29431

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: kurmikon, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham added a comment.


  Thanks!

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon added a comment.


  In D29431#669402 , @ngraham wrote:
  
  > Got confirmation from @davidedmundson that this is okay. Can you provide an 
email address so I can land your patch with correct git authorship information?
  
  
  kurmi...@libero.it

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham added a comment.


  Got confirmation from @davidedmundson that this is okay. Can you provide an 
email address so I can land your patch with correct git authorship information?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  That's not how it works. Patches need to be properly reviewed. I can do so on 
behalf of VDG, bud I'd like @davidedmundson's sign-off too, because IIRC he was 
the one who proposed or implemented this.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon marked an inline comment as done.
kurmikon added a comment.


  @ngraham No reply after one week. I think this can be merged.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham added a comment.


  @davidedmundson are you okay with this?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon updated this revision to Diff 82001.
kurmikon added a comment.


  Removed stepsize property, default value 1.

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29431?vs=81946&id=82001

REVISION DETAIL
  https://phabricator.kde.org/D29431

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham added a comment.


  There's some merit to your argument, yeah.

INLINE COMMENTS

> main.qml:295
>  id: dpiSpinBox
> -stepSize: 24
> +stepSize: 1
>  editable: true

that's the default value, so you can just remove this line entirely

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon added a comment.


  In D29431#663581 , @ngraham wrote:
  
  > Maybe we went too far in the direction of efficiency at the expense of 
familiarity, because the 24px step size suggests inaccurately that intermediate 
values can't be entered?
  
  
  That's the point. This option is an advanced setting, disabled by default, 
that the user will enable to set a specific value already in mind. There's 
nothing to navigate in my opinion. It's also an option to be set accurately, so 
introducing a stepsize may give the wrong message that intermediate values 
can't be set.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Nathaniel Graham]

ngraham added reviewers: Plasma, VDG.
ngraham added a comment.


  IIRC the idea was that people who have specific DPI in mind would type it in, 
and clicking on the arrows to navigate one at a time would be 
frustrating--hence the large step size. Maybe we went too far in the direction 
of efficiency at the expense of familiarity, because the 24px step size 
suggests inaccurately that intermediate values can't be entered?

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon, #plasma, #vdg
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon added a comment.


  In D29431#663558 , @davidedmundson 
wrote:
  
  > That doesn't explain why we're changing the step size.
  >
  > No-one wants to click the up arrow 100 times.
  
  
  I have to set 101 and the default is 96. The arrow are there but I can't set 
101 because of the stesize. Tell me if this is sane. The majority of monitors 
have DPI around 96. Obviously, if you want an higher value, you can set it 
manually.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[David Edmundson]

davidedmundson added a comment.


  That doesn't explain why we're changing the step size.
  
  No-one wants to click the up arrow 100 times.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

To: kurmikon
Cc: davidedmundson, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D29431: Remove stepsize 24 from DPI spinbox inside kcm fontconfig

[Giusy Margarita]

kurmikon created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
kurmikon requested review of this revision.

REVISION SUMMARY
  Since the spinbox read and write the DPI value "as is", without performing 
any format change, there's no need to set a stepsize of 24.
  
  As reported here 
, 
xdpyinfo, xrdb and fontconfig does not expect values multiple of 24 and 
sometimes the proper DPI value is an unsigned integer different than the 
default 96, to be manually calculated because xorg can't auto determine it 
correctly (see here 
).

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D29431

AFFECTED FILES
  kcms/fonts/package/contents/ui/main.qml

To: kurmikon
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart