ngraham requested changes to this revision.
ngraham added a comment.

  In general +1, let's not let this tiny UI improvement die.
  
  However the implementation is not quite finished: Whenever you set a 
rightMargin to make something look better next to a scrollbar, you need to 
conditionalize that on it being a LTR language, and add a matching version for 
the leftMargin when using an RTL language, because in that case, the scrollbar 
goes on the other side of the view. Basically like this:
  
    Layour.rightMargin: LayoutMirroring.enabled ? 0 : 
Kirigami.Units.smallSpacing / 2
    Layout.leftMargin:  LayoutMirroring.enabled ? Kirigami.Units.smallSpacing / 
2 : 0
  
  Alternatively if the items are inside a Layout--as they are here--instead of 
using margins, you can add an `Item` to the layout with the width of the 
spacing you want, and RTL layouts will look fine automatically. This works 
because the direction of items in a Layout get automatically reversed for RTL 
languages, while the same is not true of manually-set margins.
  
  Your choice; I think using an `Item` is a bit more elegant but I don't object 
to the alternative approach.

REPOSITORY
  R124 System Settings

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

To: filipf, #vdg, ngraham, davidedmundson
Cc: abetts, davidedmundson, plasma-devel, LeGast00n, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

Reply via email to