[Kickoff] Fix RTL issues
ClosedPublic

Authored by safaalfulaij on Jan 10 2018, 10:47 AM.

Details

Summary

Fix 2 issues:

  • the breadcrumbs alignment
  • the alignment of English entries
Test Plan

Tested and working for both LTR and RTL layouts

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
safaalfulaij created this revision.Jan 10 2018, 10:47 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 10 2018, 10:47 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
safaalfulaij requested review of this revision.Jan 10 2018, 10:47 AM

After applying:

Sorry couldn't crop video

broulik added inline comments.
applets/kickoff/package/contents/ui/ApplicationsView.qml
124

Where is it "already mirrored"? I think we're just hiding a different issue here. Aren't you actually mirroring the text here?

I'll answer that here to put some visuals.

This is before the patch, all fine, correct direction, but aligned to left:

Here we mirror the graphics of the Flickable:

And then we mirror back the graphics of the first item (I work on a small testing version):

We notice the order is reversed (becuase we already mirrored the whole flickable, and the Row is applying mirroring here, so it's double mirroring). That is why we disable the mirroring of the row:

And mirror the graphics of the other items (double mirroring again, but to our benefit):

I see.

How about playing with leftMargin of the Flickable to right-align it? Something along the lines of

leftMargin: Math.max(0, width - contentWidth)

I see.

How about playing with leftMargin of the Flickable to right-align it? Something along the lines of

leftMargin: Math.max(0, width - contentWidth)

Well, that is actually great, and I was about to update the patch to have only the alignment fixing, but I found one thing.
While entering deep in the list and the flickable should start working (moves things because of space shortage), it doesn't. The items stay as they are currently without moving. I think I'm missing something but not sure. Since you know more can you check please? :)

Perhaps you could manually set contentX to an appropriate value after adding a breadcrumb. I think the issue is that Flickable internally is still left-aligned, so I would suggest meddling around with contentX

  • Just align it correctly
  • Align header labels to left

Looking good!

applets/kickoff/package/contents/ui/ApplicationsView.qml
112

leftMargin does not have a RESET property, ie. you cannot assign undefined to it. Just set it to zero instead.

138–142

You can turn it around, also use braces even for single line statements

if (LayoutMirroring.enabled) {
    ...
} else {
    ...
}
  • 0 instead of undefined
  • change statments order
broulik accepted this revision.Jan 15 2018, 10:49 AM
This revision is now accepted and ready to land.Jan 15 2018, 10:49 AM
This revision was automatically updated to reflect the committed changes.