Fix right-to-left layout in SearchBar

[asashnov]

There was a hacky implementation which created QPushButton as a child of QLineEdit, but without using any Layout (actually, QLineEdit has no documented facilities to contain any other widget). So, somehow in old implementation Qt placed QPushButton in the left side of QLineEdit. QLineEdit itself didn't knew about it, so in CSS left padding 40px; was needed. With '-reverse' CLI option (right-to-left layout) padding 40px remained to be on the left. Also, depending on which text we enter in QLineEdit (English or Arabic) the text can also be started from the left or the right side of text area in QLineEdit, so otherwise we had to detect text layout and update CSS paddings on the fly.

The new implementation is more elegant and uses the documented facility of QLineEdit to contain an QAction. So no hacks with CSS padding is need. Also, because QPushButton was a separate QWidget, clicking on it didn't pass the focus into QLineEdit, but clicking on QAction icon does focusIn. So focus interation code also has to be changed.

Fixes: #594

[asashnov]

Testing notes:

In Search mode 'Search' is on the left, but this only because 'Search' is in Enlish:

When run with 'ar' (Arabic) LANG the 'Search' is in Arabic, and also right-to-left:

[veloman-yunkan]

@kelson42 I added you as a reviewer so that you can verify that the change is functionally acceptable. I will review the code after you confirm that it works fine.

[asashnov]

@kelson42

@asashnov Thx for your PR, this is a lot better, but IMO there is problem with the alignment of content within the searchbox itself, see my comment at https://github.com/kiwix/kiwix-desktop/issues/594#issuecomment-1050797097 :

The context of the search box:

• The text should be aligned right (like the suggestions)
• Any magnify logo should be on the right as well

When run with LANG=ar (Arabic, no -reverse CLI option is needed) content of search box is already as described:

[asashnov]

-reverse changes the layout, but keeps menu texts in English. So with just -reverse option we cannot see fully mirrored controls because strings in English continue to be aligned left (in context menus for example). This is why LANG=ar is better for testing this PR

I've seen a lot of work in past on this. Indeed, it's a trick to make QLineEdit to change the icon inside it accordingly. I tried to keep my changes small, but the over all code readability decreased a lot, so I decided to make it more easy to read in future. I ordered the code from top to bottom as it executed in runtime, and commented.

@kelson42 the issue with quick typing- did you test on MacOS?

[kelson42]

@asashnov No, I tested on Ubuntu, this code repo does not run on macOS.

[kelson42]

@asashnov Any news?

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[asashnov]

This is why we need to connect the signal every time:

From Qt sources, qlineedit.cpp:

void QLineEdit::focusInEvent(QFocusEvent *e)
...
    if (d->control->completer()) {
        d->control->completer()->setWidget(this);
        QObject::connect(d->control->completer(), SIGNAL(activated(QString)),
                         this, SLOT(setText(QString)));
...

void QLineEdit::focusOutEvent(QFocusEvent *e)
... 
    if (d->control->completer()) {
        QObject::disconnect(d->control->completer(), 0, this, 0);
    }

[asashnov]

This is a completely new fix, not based on the previous changes I made.

This change is trying to be minimal. And it use reliable way of distinguishing between a click on action icon, or inside the text area inside QLineEdit by calling QApplication::widgetAt() (it returns the widget for action when clicked on action button).

So this PR should not have any side effects.

Also, added a comment why we need to connect every time QCompleter::activated() signal.

[kelson42]

@asashnov Thx for the new attempt. Will check within a few days.

[kelson42]

@asashnov From a testing POV it looks good to me! @mgautierfr Could you please make the technical code review?

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[kelson42]

@mgautierfr Can you review again this PR?

[asashnov]

rebased on current master

[kelson42]

@mgautierfr please review

[mgautierfr]

I'm not sure about the functional part. When I run this branch with LANG=ar kiwix-desktop have got this: The search icon "glass" is on the left where it should be on the right. If click on searchbar, the cursor is also on the left and my search text is also on left.

Shouldn't we have everything on right ?

[kelson42]

@asashnov It seems that @mgautierfr is right... in the first version of your PR this was right, see https://github.com/kiwix/kiwix-desktop/pull/797#issuecomment-1046198653

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[asashnov]

rebased on the current master.

@mgautierfr Strange indeed that in your experience LANG=ar didn't have the effect. I specified LANG value in QtCreator's program run environment:

In the current version of code the placement of user interface items is totaly up to Qt, so once Qt configured correctly it will place UI items in most appropriate way for the current language.

When run as specified above, the placement of controls is right-to-left:

[kelson42]

@mgautierfr Don't forget to review please

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[kelson42]

@mgautierfr Still waiting your review. If only a few details have to be fixed, please carry one yourself. This PR is really old meanwhile.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

[stale[bot]]

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.