Code review comment for lp:~mzanetti/unity8/launcher-sizing

Revision history for this message
Michael Terry (mterry) wrote :

I know this may be bending too far backwards, but have you considered re-using the unity7 gsettings locations for the launcher?

The org.compiz.unityshell schema, installed by the unity-schemas package, has a launcher-hide-mode key and a icon-size key. The former should work just fine for our needs. But the icon-size is specified in pixels. So if we used that key, maybe we'd do some clever (or dumb!) converting.

I know it's cleaner to use our own schema. But compatibility between the two DEs (both of which we make!) would be nice. But I'm not going to push very hard for it. :)

===

In Launcher.qml, when lockedVisible changes, this code immediately calls hide(). Do you want to trigger the dismiss timer instead? That way panel.preventHiding gets respected. And just calling hide() assumes that lockedVisible is tied to autohideEnabled. Which it is... But feels weird to have those be overlapping states and have Shell.qml be the one combining them.

How would you feel about renaming lockedVisible to something like visibleByDefault and just having it be set to "scenario == desktop"? And then having Launcher combine that state and autohideEnabled to do the right thing.

The current way is fine too. But maybe just use the dismiss trigger instead of hide() at least.

===

I also tested this on my phone and couldn't pull the launcher out. It just didn't come out when I swiped from the left... I probably did something wrong... Maybe I should do a fresh build, I just moved qml files into place (no c++ changes here or in pre-req, right?) since the debs from jenkins are 404 now. Have you tested this MP recently?

review: Needs Information

« Back to merge proposal