fix RTL appearance for ComboBox
ClosedPublic

Authored by mvourlakos on Dec 9 2017, 10:18 PM.

Details

Summary

--PlasmaComponents3.ComboBox now appears correctly
its list items in RTL environments by aligning them
to the right

BUG: 387558

Test Plan

check the combobox list both in RTL and LTR environments
in order to confirm that the list items are aligned correctly

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mvourlakos created this revision.Dec 9 2017, 10:18 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptDec 9 2017, 10:18 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
mvourlakos requested review of this revision.Dec 9 2017, 10:18 PM
mvourlakos updated this revision to Diff 23708.Dec 9 2017, 10:21 PM
  • fix typos
mvourlakos edited the summary of this revision. (Show Details)Dec 9 2017, 10:27 PM
mvourlakos edited the test plan for this revision. (Show Details)

just noticed that the reviewer is the person that decides to give the green light so it cant be set by the author without prior communication...

@mart what about this? are you able to reproduce it in your system?

mart accepted this revision.Jan 12 2018, 3:52 PM
This revision is now accepted and ready to land.Jan 12 2018, 3:52 PM
This revision was automatically updated to reflect the committed changes.

Can you please explain why these are needed? I tested plasmacomponents3 and the combobox was working correctly.

Can you please explain why these are needed? I tested plasmacomponents3 and the combobox was working correctly.

It wasnt, use english text in the list items to see the issue without the patch.

safaalfulaij added a comment.EditedJan 13 2018, 9:05 PM

Can you please explain why these are needed? I tested plasmacomponents3 and the combobox was working correctly.

It wasnt, use english text in the list items to see the issue without the patch.

Nope, I have also English text in the model. Its:
تجربة_
_test
test

Can you please explain why these are needed? I tested plasmacomponents3 and the combobox was working correctly.

It wasnt, use english text in the list items to see the issue without the patch.

Nope, I have also English text in the model. Its:
تجربة_
_test
test

how did you test it?

I tried in an qml app (Latte dock) by passing the parameter "--reverse"...
If your system is already using in RTL language, have you tried with --reverse?

how did you test it?

I tried in an qml app (Latte dock) by passing the parameter "--reverse"...
If your system is already using in RTL language, have you tried with --reverse?

I am using this with qmlsence-qt5.
plasma folder is the ones before this, and plasmacomponents3 is the one after this.
Using C locale, everything aligned to left. Using --reverse will align everything to right.
Using ar locale, everything aligned to right. Using --reverse will not reverse it to LTR layout and it makes everything aligned to left.

Can't see the problem really.

import QtQuick 2.7
import QtQuick.Layouts 1.3
import QtQuick.Controls 2.0
import "plasma" as Plasma
import "plasmacomponents3" as PlasmaNEW

ApplicationWindow {
    LayoutMirroring.enabled: Qt.application.layoutDirection == Qt.RightToLeft
    LayoutMirroring.childrenInherit: true
    RowLayout {
        Plasma.Button {
            id: fileButton
            text: "File"
            onClicked: menu.open()
            
            Plasma.Menu {
                id: menu
                y: fileButton.height
                x: fileButton.width - width
                
                Plasma.MenuItem {
                    text: "New..."
                }
                Plasma.MenuItem {
                    text: "Open..."
                }
                Plasma.MenuItem {
                    text: "Save"
                }
            }
        }
        
        Plasma.Label {
            text: "2"
        }
        Plasma.ComboBox {
            model: ["أولا_", "Second_", "Third"]
        }
        PlasmaNEW.ComboBox {
            model: ["أولا_", "Second_", "Third"]
        }
        Plasma.ProgressBar {
            value: 0.3
        }
        Plasma.Switch {
            text: "Switch"
        }
    }
}

how did you test it?

I tried in an qml app (Latte dock) by passing the parameter "--reverse"...
If your system is already using in RTL language, have you tried with --reverse?

I am using this with qmlsence-qt5.
plasma folder is the ones before this, and plasmacomponents3 is the one after this.
Using C locale, everything aligned to left. Using --reverse will align everything to right.
Using ar locale, everything aligned to right. Using --reverse will not reverse it to LTR layout and it makes everything aligned to left.

Can't see the problem really.

with your example me I cant reproduce it either...
I can only reproduce it with Latte Settings window, all other components are aligned correctly except the PlasmaComponent3.Combobox list items.

you are right, this might be a downstream bug...

Do you want to revert it
I can do it

with your example me I cant reproduce it either...
I can only reproduce it with Latte Settings window, all other components are aligned correctly except the PlasmaComponent3.Combobox list items.

you are right, this might be a downstream bug...

Do you want to revert it
I can do it

I checked out the source of latte-dock and it seems that it is using the old QQC1-based controls, which are problematic, especially with RTL :)
This is a change for QQC2-based controls (version 3.0 of org.kde.plasma.components).

But I can't understand how changing this V3 solved V2..

with your example me I cant reproduce it either...
I can only reproduce it with Latte Settings window, all other components are aligned correctly except the PlasmaComponent3.Combobox list items.

you are right, this might be a downstream bug...

Do you want to revert it
I can do it

I checked out the source of latte-dock and it seems that it is using the old QQC1-based controls, which are problematic, especially with RTL :)
This is a change for QQC2-based controls (version 3.0 of org.kde.plasma.components).

To test it in Latte I added in the head of /shell/package/contents/configuration/AppearanceConfig.qml

import org.kde.plasma.components 3.0 as PlasmaComponents3

and I set up the ComboBox in that file to:

PlasmaComponents3.ComboBox

But I can't understand how changing this V3 solved V2..

V2 components are based totally at Qt Styles, V3 are reimplementing the behavior
PlasmaComponents2.ComboBox is a ComboBoxStyle but
PlasmaComponents3.ComboBox is a real ComboBox

To test it in Latte I added in the head of /shell/package/contents/configuration/AppearanceConfig.qml

import org.kde.plasma.components 3.0 as PlasmaComponents3

and I set up the ComboBox in that file to:

PlasmaComponents3.ComboBox

But I can't understand how changing this V3 solved V2..

V2 components are based totally at Qt Styles, V3 are reimplementing the behavior
PlasmaComponents2.ComboBox is a ComboBoxStyle but
PlasmaComponents3.ComboBox is a real ComboBox

Did you test this with your LTR layout with only --reverse?
I can say now that the popup box (that is a Popup) takes its direction from the locale used and not from the application, since it doesn't inhert Item, and LayoutMirroring doesn't affect it

It is similar to this issue with the Slider.
I'll add more info to that soon.

src/declarativeimports/plasmacomponents3/ComboBox.qml
113

So it is true even for LTR locales?

@safaalfulaij @mart we need to revert this...
with frameworks 5.43 it reverses list items always!!!

The issue is still present when the PlasmaComponents3.ComboBox is used in the Latte settings window,
that is the items are not aligned correctly in --reverse mode

src/declarativeimports/plasmacomponents3/ComboBox.qml
113

yes,
in my system with these settings the combobox list items are aligned correctly for both
LTR and --reverse

@mart I fixed this issue by changing these lines to:

113            LayoutMirroring.enabled: Qt.application.layoutDirection === Qt.RightToLeft
114            LayoutMirroring.childrenInherit: true

@mart I fixed this issue by changing these lines to:

113            LayoutMirroring.enabled: Qt.application.layoutDirection === Qt.RightToLeft
114            LayoutMirroring.childrenInherit: true

Would you please add a HACK so that we know when to remove this? This is because Popup doesn't inherit LayoutMirroring and bases the direction according to the locale used, exactly as in the Qt bug report I've mentioned above.

In normal conditions, where language = locale, this is not needed. But in cases where they are different, the locale direction is preffered for Popup, and therfore this will be needed.

Would you please add a HACK so that we know when to remove this?

You mean as a comment ?

Would you please add a HACK so that we know when to remove this?

You mean as a comment ?

Yes, preferably with the Qt bug number and some explanation.

I've done some more testing and read some Qt docs. Popup item get reparented to the window once displayed, so LayoutMirroring is applied to it only at that point (applying it to the contentItem).
So my comments above about Popup not inheerting Item are wrong, sorry.
I'm still not sure if this is needed really.

Can you build Latte from master version? All comboboxes in its settings window are PlasmaComponents3

Can you build Latte from master version? All comboboxes in its settings window are PlasmaComponents3

:)
Ok, I can assume that Latte doens't have a Window/ApplicationWindow, and that is why this is needed.
I've tried this and it didn't work as with ApplicationWindow instead of the base Item:

import QtQuick 2.7
import QtQuick.Layouts 1.3

import "plasma" as Plasma

Item {
    LayoutMirroring.enabled: true
    LayoutMirroring.childrenInherit: true

    width: 350
    height: 150

    RowLayout {
        anchors.fill: parent
        Plasma.ComboBox {
            id: combo
            model: ["أولا_", "Second_", "Third"]
        }
    }
}

We should include this for now, with the HACK comment, till we can get it somehow solved in Qt.
Sorry for all the misunderstandings!

Reported: https://bugreports.qt.io/browse/QTBUG-66446
Although this is really unlikly to happen, a widget having a combobox, but this is the case of Latte where the whole config overlay is just a MouseArea.

Latte config window is a Plasma::ConfigView, so it inherates a QQuickView