Differential D15308

allow ViewSelector to show icon only when the window is small
ClosedPublic
Actions

Authored by mgallien on Sep 6 2018, 6:36 AM.

Details

Reviewers
ngraham
astippich
januz
Commits
R255:152cbb81c080: allow ViewSelector to show icon only when the window is small
R255:34dc3e309209: allow ViewSelector to show icon only when the window is small
Summary

When the elisa window is small, ViewSelector only show icons and
the entry text is shown only via a tooltip

CCBUG: 392778

T9640

Test Plan

allow to start to fix issues when quarter tiled

Diff Detail

Repository
R255 Elisa
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mgallien requested review of this revision.Sep 6 2018, 6:36 AM
mgallien created this revision.
astippich requested changes to this revision.Sep 6 2018, 7:26 PM

I like the idea, but it still shows the elided text with about 3 visible characters.

src/qml/ContentView.qml
860

please put this value into the theme file. makes it easier to change it later.

src/qml/ViewSelector.qml
246

Does such a hardcoded value work with HighDPI?
I think setting the state externally from the ContentView is more reliable

This revision now requires changes to proceed.Sep 6 2018, 7:26 PM
mgallien planned changes to this revision.Sep 8 2018, 6:38 AM
In D15308#321502, @astippich wrote:

I like the idea, but it still shows the elided text with about 3 visible characters.

Thanks for your review. It made me think about it more. I plan to do changes to the way this patch is done. I will try to ensure that the transition happen before only 3 characters are shown.

mgallien updated this revision to Diff 41187.Sep 8 2018, 7:12 AM
  • ViewSelector resize according to the maximum width set from ContentView

try to take into account the review from @astippich by only handling the
resize inside ViewSelector without having ContentView managing states

mgallien marked 2 inline comments as done.Sep 8 2018, 7:13 AM
astippich requested changes to this revision.Sep 11 2018, 6:22 PM

Working great now!
The windows theme file changes are missing and two comments inline

src/qml/Theme.qml
97 ↗(On Diff #41187)

this seems unused now

src/qml/ViewSelector.qml
32

Can this somehow be determined automatically depending on the text? One would have to look for the longest entry. This would also adjust to different font sizes and different languages then.

This revision now requires changes to proceed.Sep 11 2018, 6:22 PM
mgallien added a comment.Sep 11 2018, 8:07 PM

Thanks again for your review.
The fact that everything has to be duplicated is a conception mistake. I will open a review to fix that.

mgallien planned changes to this revision.Sep 11 2018, 8:16 PM

Once D15437 is landed, I will update this one on top of it.

mgallien updated this revision to Diff 41570.Sep 13 2018, 7:06 PM
  • ViewSelector resize according to the maximum width set from ContentView
  • ensure that the filter part in NavigationActionBar can become small

rebase on top of 0.3 branch

astippich added a comment.Sep 13 2018, 8:15 PM

Still the comments inline, and the rebase was not complete? it has theme changes included

src/qml/BaseTheme.qml
117

still unused

src/qml/NavigationActionBar.qml
275

unnecessary

mgallien added inline comments.Sep 13 2018, 9:06 PM
src/qml/NavigationActionBar.qml
275

That would be an unrelated change. I can clean it up in a later commit.

mgallien updated this revision to Diff 41587.Sep 13 2018, 9:07 PM
  • ViewSelector resize according to the maximum width set from ContentView
  • ensure that the filter part in NavigationActionBar can become small
  • remove unused property in BaseTheme component

fix one issue repoted by astippich

mgallien added inline comments.Sep 13 2018, 9:09 PM
src/qml/ViewSelector.qml
32

It is just a dummy big value needing to be higher than the maximum width set by the container.

mgallien marked 3 inline comments as done.Sep 13 2018, 9:10 PM
astippich accepted this revision.Sep 17 2018, 5:05 PM
astippich added inline comments.
src/qml/NavigationActionBar.qml
275

You add an unrelated new line here, but nevermind

src/qml/ViewSelector.qml
32

Is that also true for 4k monitors? that said, can probably be addressed later

This revision is now accepted and ready to land.Sep 17 2018, 5:05 PM
Closed by commit R255:34dc3e309209: allow ViewSelector to show icon only when the window is small (authored by mgallien). · Explain WhySep 17 2018, 6:34 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 41861

View Options

src/qml/BaseTheme.qml

Loading...
View Options

src/qml/ContentView.qml

Loading...
View Options

src/qml/NavigationActionBar.qml

Loading...
View Options

src/qml/ViewSelector.qml

Loading...