linuxmint / cinnamon

A Linux desktop featuring a traditional layout, built from modern technology and introducing brand new innovative features.
GNU General Public License v2.0
4.4k stars 726 forks source link

[3.2] Vertical panels #4996

Closed brownsr closed 7 years ago

brownsr commented 8 years ago

Vertical panels

Thanks for comments and suggestions/corrections from Dalcde, JosephMcc, Lestcape and mtwebster

What this does

• Horizontal and vertical panels can be mixed freely • Horizontal panels take precedence • User functionality essentially unchanged. • Panels display in bottom, side, top sequence so that shadow from one panel falls naturally on another • Panels can be moved freely between horizontal and vertical formats • Applets have to declare their suitability for placement in a vertical panel. This gives protection against 3rd party applets that are not coded to fit a vertical panel. • Long applets like the panel launcher, the systray and the workspace switcher orient themselves correctly on vertical panels, and reorient automatically when dragged. • CSS has been added to the default theme for left and right panels, boxes in vertical panels, panel launcher, window-list, the separator, and the general applet-box. Code uses the 'important' property for the new CSS so that existing themes should function. • Margin can be added to the CSS so that panels can be indented at their ends • If you mix horizontal and vertical panels the popup menus adjust to keep from covering any panel

The menu applet suppresses its label in a vertical panel, as it will not fit. Calendar date display format is set to %H%n%M in a vertical panel, as it is all that will fit with a typical default font.

Note that there are some generally suitable applets that will need some simple configuration to look right, these are ones that present themselves as IconApplets but put additional information out on the screen. For example the popular 3rd party 'weather' applet will need the additional panel fields for temperature and condition turned off

What this does not do

Dynamic resizing of vertical panels when horizontal panels hide themselves Changes to mint themes other than the default cinnamon theme. These are in separate PRs

Current Position

Stable, merged. Known bugs:

Right to left text untested, as I do not read any languages of this nature.

Pre-existing bugs, still there:

Corner display and autohide do not play nicely with each other. Issue there in in 17.3 as well. In the production version the corner eats into the panel (http://imgur.com/6GsDyYo) as well as appearing to not be functional on bottom panels, and on redisplay everything except the proportion that eats into the panel is lost on redisplay. In this PR the corner is all outside the panel and this is all lost on redisplay at the moment.

Notes for 3rd party applet designers

Applets for placing in a vertical panel have to fit. In practice that means there is space for an icon, nothing else. Placing larger applets in a vertical panel disturbs the alignment of all other applets in that part of the panel, and it all will look very broken, so protection has been added to guard against that.

Applets based around IconApplet are assumed to be suitable for vertical panels and can be placed there. However if you maintain one of these then please do check they work OK - your applet may write additional information out to the screen that Cinnamon does not know about when the applet is loaded. This will mess up the central alignment unless suppressed.

All other applets need an explicit declaration in their code that they work in a vertical panel. If this is not done then a user will simply not be able to place them there from the applet manager, or by drag and drop in panel edit mode, and they will be turned off if moving a whole panel from a horizontal to a vertical position.

The mechanism for doing this is very simple, just place a call in your code:

    this.setAllowedLayout(Applet.AllowedLayout.BOTH);

Any sophisticated applet may well need logic that has different actions dependent on the type of panel it is in, so would need a call coded like this:


    on_orientation_changed: function (orientation) {
        this.orientation = orientation;

        if (this.orientation == St.Side.LEFT || this.orientation == St.Side.RIGHT) { // vertical
       ...
       } else { // horizontal
        ...
       }
...
    }

If you have an applet with a label that needs suppressing in a vertical panel, you will need code that explicitly shows or hides the label like this:


    on_orientation_changed: function(orientation) {
        if (orientation == St.Side.LEFT || orientation == St.Side.RIGHT)
            this.hide_applet_label(true);
        else
            this.hide_applet_label(false);
    },

If you have long multi-element applets like the window list, the panel launcher, the workspace switcher or the systray then it is likely you will have to intermediate a management container that can facilitate switching between horizontal and vertical modes. Take a look at the code in these standard applets as a start (github: Cinnamon/files/usr/share/cinnamon/applets) If you have an applet that changes its dimension then take a look at the spacer applet code for a starting point.

Where different CSS is needed for vertical and horizontal presentations then this has been done by including .vertical additional CSS. Again, take a look at the panel launcher or window list standard applets for how to use this - add 'vertical' style in a vertical panel, take it off again in a horizontal panel.

One last thing is to ensure that your code uses the current parameters and calls to init(). If your code does not include the panel height and orientation parameters and use them in an init call then it is likely your applet will a) come up the wrong size when added until the next Cinnamon restart b) orient off-centre in a vertical panel, until the next Cinnamon restart c) have sub-standard popup menu alignment . Take a look at the stock applets in github: Cinnamon/files/usr/share/cinnamon/applets if in doubt.


MyApplet.prototype = {
    __proto__: Applet.IconApplet.prototype,

    _init: function(metadata, orientation, panel_height, instance_id) {
        Applet.IconApplet.prototype._init.call(this, orientation, panel_height, instance_id);

Notes for theme designers

A number of .vertical CSS style variants had to be added, and there are a few cases in the 'panel' CSS where reference by name (#) had to be replaced with the css style (.) reference. In general there are not many changes though, and you can see the master set in the default cinnamon theme (github: Cinnamon/data/theme/cinnamon.css)

If you want to make something suitable for a vertical panel, the key thing is to avoid margin/padding to the left and right if at all possible - things in a vertical panel look best centred.

The boxes in a vertical panel are styled panelLeft, panelCenter, and panelLeft for the left panel (top to bottom order) and panelRight, panelCenter, panelRight for the right panel. This is because most themes are symmetric in the way they treat the two panel boxes either side of the central one.

The .panel-left and .panel-right css is inspected for margin-top and margin-bottom, and the left and right panels are shifted in top and bottom accordingly, as I had requests to allow vertical panels to be shifted inwards from the top and bottom. For consistency the same was done for margin-left and margin-right for horizontal panels.

zenfuzz commented 8 years ago

Yes!

JosephMcc commented 8 years ago

@brownsr I had my first chance to try this out.

I use a dual monitor set up and ran into issues right of the bat. Not sure if you have the hardware to test by I started a branch here: https://github.com/JosephMcc/Cinnamon/commits/vertical-panel-fixes

It contains a fix that was preventing panels from being loaded properly on all monitors. The other commit is just clean up to bring it more in line with other cinnamon js code and make it a bit easier to work with. Feel free to grab the changes for your branches.

The other thing that I see just looking through the code is how style classes are handled. Currently you are using completely different classes when an applet is horizontal vs. vertical. Instead of doing that use add_style_class_name('vertical') and remove_style_class_name('vertical'). The advantage to that is the vertical applets will inherit all the styling from the normal ones. If you need changes you can just override the specific properties you want to change. Make for easier/cleaner theming.

These warnings:

(cinnamon:2738): St-WARNING **: Did not find color property '-arrow-background-color'

(cinnamon:2738): St-WARNING **: Did not find color property '-gradient-start'

(cinnamon:2738): St-WARNING **: Did not find color property '-gradient-end'

are caused by the specific theme you are using and not caused by your code.

brownsr commented 8 years ago

Thanks very much. I'll work through that.

zenfuzz commented 8 years ago

Thanks, guys.

Mowalle commented 8 years ago

Thank you for working on this. This is one of the features that I really miss.

Regarding your comment about the calendar applet:

Unsure what to do about the calendar applet. Maybe a vertical panel could show a basic HH24:MI clock regardless of what date format is actually set - because anything longer will not fit, and still be legible.

Can't the user simply put a %n into the format to create a newline? That's at least how you can handle the datetime applet on verticle panels in xfce.

brownsr commented 8 years ago

Thanks Mowalle, I didn't know about that. I've been spending far too much time on this relatively small thing trying to devise scaling workarounds that simply do not work or trigger strange bugs. And in 2 mins with your advice there's a workable way through :)

Azureit commented 8 years ago

Thanks for your work on this. This is the only thing that is stopping me from moving from mate to cinnamon. I like the panel on the right, size 32px, like this: 1- Menu icon 2- Clock <span font="Digital Numbers 12" color="#ff8080"><b>%H</b></span>\n<span font="Digital Numbers 8" color="#6699ff"><b>%M</b></span> 3- Systray 4- Windows list(In this case mate-applet-dock) 5- Trash icon Screenshot my desktop

ghost commented 8 years ago

@brownsr I suspect that the migration to vertical panel for third party applets will be slow and gradual. There are then, some applets that will remain incompatible with the new code. In my opinion, will be nice if there are a filter function to not add an incompatible applet inside the vertical panel, i.e. if the applet not declared explicitly that he can display his components in that way.

Probably this will help to stop a lot of user "issues" about this. Maybe, this can also be stopped with a comment inside the release note of cinnamon, or maybe in others ways. Anyway, there are a lot of applets currently that not support the last cinnamon version, as my own applets.

brownsr commented 8 years ago

Appreciate the comment lestcape. I don't know of any way of doing this other then testing every applet and blacklisting the ones that have problems, and re-testing periodically to reduce the blacklist. That would cause its own problems. If you can suggest any generic way then I can give it a go.

I took the systray changes from your PR to make this PR self-contained. As well as the filter, there was that infinite loop you had corrected, that got triggered when inserting a layout manager in the hierarchy.

mtwebster commented 8 years ago

The way I always envisioned this working was:

I'd prefer to avoid any sort of blacklist, and instead provide a set of rules within the applet types to ensure good behavior.

ghost commented 8 years ago

In my point of view i will do that:

appletManager.js

const DisplayLayout = {
    VERTICAL: 'vertical',
    HORIZONTAL: 'horizontal',
    BOTH: 'both'
}

applet.js

   function getDisplayLayout() {
        return AppletManager.DisplayLayout.HORIZONTAL;
   }

myapplet.js

function MyApplet() {
   this._init.apply(this, arguments);
}

MyApplet.prototype = {
   __proto__: Applet.Applet.prototype,
   .......
   //override getDisplayLayout
   function getDisplayLayout() {
        return AppletManager.DisplayLayout.BOTH;
   }
   .....
}

Then you need to compare the DisplayLayout of an applet instance with the panel layout and allow or deny the drop action inside the panel.

Please see that you can change the code inside of applet.js and apply the @mtwebster suggestion, reeding the layout behaviour for the metadata.json and returned the value or also checked if is an icon Applet first and only in this case return AppletManager.DisplayLayout.BOTH without read the metadata.json

But that's this is my idea, is not necessary the best one.

ghost commented 8 years ago

Additional to this fact, we need to check on applet initialization if there are a supported panel where then be added or not. And not add an applet to a non supported panel. A cinnamon update with this new code, without a check of this, could be in my opinion dangerous.

ghost commented 8 years ago

And when panel is moved to any horizontal position from any vertical or vice versa, also is needed a filter from the applet that do not satisfy the new type of the panel position and remove this applet from panel or move the applet to a allowed panel if there are one.

brownsr commented 8 years ago

I gave a quick test to a batch of 3rd party applets, working downwards in popularity. Very mixed results. Only one applet (configurable menu) caused stability problems. I was a bit surprised to find out how many were flaky even in standard horizontal panels :

weather - all OK window list with app grouping - does not display correctly configurable menu - can cause cinnamon hangs places - workable. Popup window placement comes up midway L/R on the panel and can have part of the popup window off the screen dependent on applet placement, but seems to work OK multi-core system monitor. Not really right in horizontal panels. Does not display correctly in vertical panels Desktop capture. Context menu comes up midway L/R on the panel, but otherwise appears to work OK. Needs a cinnamon reboot to size correctly on horiz panel system tray collapsible - does not display correctly places center - popup placement comes up midway L/R on the panel, but seems to work OK show desktop++ - appears broken in all panels classic menu - appears completely broken, will not install at all Better settings - needs a cinnamon reboot to size correctly on horiz panel. aligns to left on vertical panels, but otherwise OK ish. Tries and fails to fire up gnome control center with one menu item (type of panel is not relevant) News feed reader - will not install Sound with apps volume - aligns to the left in a vertical panel, and the menu comes up midway L/R on the panel but otherwise seems to walk OK Hardware monitor - disappears on a vertical panel Windows quick list with close button - works fine, though the display of icon and window count looks odd on a vertical panel. Context menu comes up midway L/R on a vertical panel Shutdown applet - alignment is to left initially when dragged into a vertical panel but otherwise seems to work fine. The screen lock item does not work on any panel Pomodoro timer - shows fine and appears to work as far as I can tell, but as it shows a timer string it will write off the side of a vertical panel and disrupt the centre alignment of other applets All in one places - doesn't exactly seem to do very much, but behaves the same in any type of panel. Context menu comes up mid-panel L/R window buttons with title - does not display correctly in vertical panel. Not that I can really work out what this is supposed to do in a horizontal panel anyway. restart cinnamon - works fine drawer - does not appear to do anything in any panel other than flip its orientation system monitor (orcus) - does not display in vertical panel timer with notifications - works just fine. System Monitor applet (ebbes) - Does not display correctly in vertical panel. Appears half broken in horizontal panel anyway Screen saver inhibit [mtwebster] - displays and works fine CPU temperature indicator -works fine, though text will burst out of side of panel and disturb the central alignment of other applets in the box Screen shot [tech71] - works fine, context menu comes up mid-panel L/R Screen shot and desktop record - works fine , context menu comes up mid-panel L/R My launcher - works fine, other than context menu placement comes up mid-panel L/R and vertical placement can be less than ideal Messaging menu - does not load Quit applet - seems to work OK other than context menu placement comes up mid-panel L/R Slingshot - works OK other than the context menu placement comes up mid-panel L/R. You have to configure the word 'menu' not to show. Force quit - works fine Stark Menu - looks broken in a horizontal panel anyway System monitor with graphs - works just fine. Gnome menu [stable] - does not install World clock calendar - works ok with custom string %H%n%M user menu - works OK, though as the user name will usually be longer than the panel it writes out beyond the panel, and disturbs the central alignment of other applets in the box Gpaste - failed to load in any panel github explorer - works OK, insofar as it works at all Sports update - seems to work fine in RH panel, though no updates came through while I used it. in LH panel the word 'live' breaks out of the panel and disturbs the central alignment of others applets in the box My bookmarks - works like a dream Window list with preview - shrinks down to a tiny size in a vertical panel

ghost commented 8 years ago

@brownsr yes, thats why i say probably with a comment in the release will be enough. Anyway the reason of have a filter is not for broken applets it's for functionals applets. See this as a feature and not as a patch. You can give for functional applets a way to prevent/allow if they can/want to be displayed vertical/and-or/horizontal.

ghost commented 8 years ago

See also we can made inside the applet an actor rotation of 90 degree in the opposite direction of the vertical panel and the support for vertical panel will not be a problem, while we search for a real solution.

ghost commented 8 years ago

@brownsr With the AppletTextIcon, i think that will be possible remove the label, as you comment inside the code. But in my opinion, will be better not remove the label in the case and only in the case that the applet reported in an explicit way that can deal with vertical panel, because for example if the label is short, can then be displayed inside the icon, as currently occurs in the WindowsIconList applet, with his special types of menu components.

brownsr commented 8 years ago

I know suppressing the label on icon+text applets works OK, as I put this in a long time ago [change removed subsequently, you and mtwebster convinced me it was inappropriate]. There's never going to be room to display the text label for one of these applets, as the icon sizes to fill most of the available space, scaling the same way as icon only applets. That was my thinking anyway. I think I agree with you for text only applets, if the author knows they will happen to come up small enough to fit. However in practice I have not found any applets that do this now. Even the calendar applet comes up with a clock that only just fits 2 digits comfortably with a typical default font size, so displaying the time needs %H%n%M. Any text applet would have to be specifically designed for the purpose I think. And then if it declares itself suitable, fine.

brownsr commented 8 years ago

OK, that's protection against unsuitable applets added. I've used the method lestcape suggested. Frankly I could not work out how to break into the extension subsystem to do it via json. AppletIcon applets are treated as OK. Each other type of applet will need a getDisplayLayout function declared if it wants to do anything other than be on horizontal panels. Dragging an applet onto an unsuitable panel simply refuses to drop. Adding an applet via the 'add applets to the panel' settings route will flag up a red button indication against the applet in the settings list. Moving a panel containing an unsuitable applet to a new orientation will make the applet disappear from the panel, and it will similarly show up with the red button indication against the applet in the settings list if launched from the panel. It does the job, but is a bit on the subtle side, however triggering the error reporting logic gives a large scary banner message telling the user to contact the applet developer, which seemed inappropriate to me.
*\ added a friendlier message when a user tries to add an unsuitable applet

collinss commented 8 years ago

You can access anything in metadata.json (which is in the directory of every applet) from the meta property of the extension object, or the _meta property of the applet object. It is read in at https://github.com/linuxmint/Cinnamon/blob/master/js/ui/extension.js#L131.

ghost commented 8 years ago

@brownsr What you say about the large scary banner message telling the user to contact the applet developer maybe is this: https://github.com/linuxmint/Cinnamon/blob/master/files/usr/share/cinnamon/cinnamon-settings/bin/ExtensionCore.py#L977

And this function is called from this place: https://github.com/linuxmint/Cinnamon/blob/master/js/ui/cinnamonDBus.js#L267

So, probably the susses could be a numeric value instead of a boolean to indicate a more specific type of loading state. Then you can response in concordance with the state.

Please see, if you want to apply that you need to change the xml of this Dbus function.

ghost commented 8 years ago

@brownsr The trace of @collinss comment: https://github.com/linuxmint/Cinnamon/blob/master/js/ui/applet.js#L163 https://github.com/linuxmint/Cinnamon/blob/master/js/ui/appletManager.js#L337 https://github.com/linuxmint/Cinnamon/blob/master/js/ui/appletManager.js#L460 https://github.com/linuxmint/Cinnamon/blob/master/js/ui/appletManager.js#L187 https://github.com/linuxmint/Cinnamon/blob/master/js/ui/extension.js#L167

mtwebster commented 8 years ago

Some initial observations testing:

I'll dive into the code some later, unfortunately I didn't have much spare time this weekend.

brownsr commented 8 years ago

Thanks for the testing feedback. I've been travelling today so have had only a little time to look at it.

The edit mode behaviour is curious, haven't been able to get to what causes it yet with some quick changes. I've gotten so used to seeing it, it stopped registering as odd. *\ have now reduced this to a specific set of circumstances, should not happen generally now

If you use a theme that has shadow like mint-x then the shadow line will visually separate butting vertical and horizontal panels, otherwise I agree they will merge visually, as currently coded. It would be simple enough to put a gap at the top and bottom of vertical panels to create a dock like effect, if that was wanted.

No idea about the monitors, will have a root around when I can.
*\ found and corrected an error which may be related, see below

Oh, and horizontal panels are dominant as coded, so a vertical panel will slot in over/under them, unless there is no horizontal panel when it will go to the edge.

JosephMcc commented 8 years ago

I feel like leaving an actual gap between the panels would be a bit strange. Themes should be able to create visual separation in a couple ways, be it shadows or borders. In fact a gap could likely be created through simple theming.

ghost commented 8 years ago

I think will be nice to have a configuration for the dominant panel layout as the comment of @mtwebster suggest.

To centering properly the middle zone of the vertical panel, i think will be interesting take a look again to this code: https://github.com/linuxmint/Cinnamon/pull/4041

mtwebster commented 8 years ago

Another quick observation, if I have a bottom panel, clicking at the bottom of it in, say, the window list applet does nothing - like a dead row there. I've seen this bug before with other panel work.. maybe theme-related? (this is on the default theme)

brownsr commented 8 years ago

With the window list there are small gaps around the representations of each window on some or all sides (pretty small with the default theme, but more obvious with others), and there may be unused panel space to either side of the window list. Nothing happens if these are clicked on.

The change I have made has intermediated a management container to handle the horizontal/vertical switching, and that changes the way the window representations work. the x/y-expand does not seem to function to drive the window representation boxes up to the edge, wherever I set it. I have now set a preferred height in horizontal panels to the height -2, which means visually that the window representation boxes go right up to the edge. However if you drag the mouse right down to the bottom you will be in a tiny gap, the highlighting shows if you are in or out of the window box, and hence whether a click will function or not. Setting to height-1 also works, but I would be concerned that this might blow up in some themes if any border/margin were set. I have not tried this though. I tried setting to panelheight, with no reduction, but this then throws up allocation errors by the bucketload.

With some irony I had documented all of this in a comment, and then forgotten it entirely. Memory of a goldfish.

The production version has no gaps to top or bottom, though it does have gaps between the window representations.

brownsr commented 8 years ago

I corrected an array error that could be triggered by having/removing an extra monitor. Symptoms were flaky context menu / edit mode zones with multiple monitors. After making this change I cannot recreate the issue mtwebster mentioned about getting the cursor trapped behind a panel barrier [unless turned on deliberately in the panel settings] other than by reaching the physical limit of the monitors [right hand edge of rightmost, top edge of topmost etc.]. I've moved monitors, set primary etc. with no issues.

Also corrected an issue that would have stopped additional 'vertical' styling being recognised for the three panel boxes.

But I am not getting anywhere finding a reason for the curious alignment when edit mode is turned on. I suspect another pair of eyes may be needed. \ found it

brownsr commented 8 years ago

Excepting the couple of oddities I have logged as bugs in the main description, I suspect the work on this is pretty much done now. I've written a set of tests, and I will try to find the time to run through it and log the results here

brownsr commented 8 years ago

Unit/regression test for vertical panels

test Expected Result Actual result
Settings module
Add new panel in settings Zones highlighted, click adds OK
Turn on edit mode in settings Edit mode turned on, relevant highlighting applied OK
Next panel in settings Highlighting applied to next panel, moves to next panel OK
Always show, hide, intellihide Show/hide behaviour of panel as expected OK
Panel size customised/theme Panel resizes according to setting OK
Scale panel text/icons Panel text/icons resize according to setting OK
Panel height scale bar Panel resizes according to bar settings OK
Panel edge cursor mode Cursor passes through panel edge according to settings still to test
Previous panel Moves to previous panel OK
General settings module
Turn on hidpi Stable hi-dpi behaviour of panel and contents OK
Turn off hidpi Stable non hi-dpi behaviour of panel and contents OK
Panel appearance
Panel appearance As defined in CSS OK
Box appearance Left, right, central as per CSS OK
Box sizing Looks balanced for various box populations OK
Makes full use of panel 'height' OK
No vertical CSS Panel uses default theme CSS OK
Standard themes Well behaved in default and Mint-X themes OK
Applet behaviour
Applet centred in panel Applet centred in vertical panel OK
(apart from CSS)
Hover Highlighted as per CSS OK
excepting systray, workspace switcher
Label Shown in a sensible place on hover OK
Context menu Shown with pointer pointing in to panel OK
Positioning of context menu is appropriate OK
for top/bottom of screen
Click Launches, animation is appropriate OK
Applet spacing Consistent and as expected OK
3rd party applets Display in vertical panel suppressed by default
if not IconApplet OK
Corners
Corners show if in CSS Corners shown OK
Colour/size as per CSS OK
Do not show if not in CSS OK
Corner alignment Diagonally inward on all panels, at end of panel OK
No panel, no corner OK
Corner design Three painted concentric arcs, OK
emphasis dot on inward top/bottom point OK
Show Corner shown on show of autohide Fail
Hide Corner hidden on hide of autohide OK
Applet/panel interaction
Applets placed on panel by dnd Applets move as expected OK
Applet dnd Dragging behaviour as expected OK
Applet dnd within panel can alter ordering readily OK
Applet display after dnd panel boxes adjust OK
Applet specifics
Panel launcher Aligns within panel on either horizontal or vertical panel OK
Window list Ditto OK
Workspace switcher Ditto OK
Systray Ditto OK
systray icons and indicators both display OK
All other supplied applets Behave sensibly on both vertical and horizontal panels OK
3rd party applets in vertical panels
First 2 pages of popular applets No desktop crashes all OK
Document behaviour bar configurable menu
Panel context menu
Add applets to panel Selected applets added to panel OK
Panel settings Panel settings shown for this panel OK
Themes Theme settings shown OK
All settings All settings window shown OK
Troubleshoot - restart Restarts OK
Troubleshoot - looking glass Looking glass shown OK
Troubleshoot - restore to default Restores to default OK
Modify - remove panel Removes panel OK
Modify - add panel Shows dummy, click adds OK
Modify - move panel Shows dummy, click moves OK
Check all potential moves cross monitor to test
Modify - copy applet config Copies config for panel OK
Modify - paste applet config Pastes config for panel OK
Modify - clear applets Clears applets on panel OK
Edit mode Toggle on/off show highlighting OK, see bugs for residual alignment issue
Multi-monitor
Panels on multiple monitors Panels place OK Still to test
Panels move between monitors Move OK Still to test
Applet behaviour as expected All applet behaviour as expected Still to test
Save/restore
Stable panel across restart Restart shows panels as they were OK
Interacting panels
Panel overlap None OK
Preference for horizontal panels Vertical panels resize to avoid horizontal OK
Panel shadow Falls naturally OK
Internals
Occasions when log warnings are triggered none OK
Quirks
Non-intuitive behaviour encountered Over-wide applets cause alignment to centre of applet set rather than the panel - a good reason to suppress over-wide applets

Note: multi monitor tests shown as still to be done, but in practice I expect them to be OK. I gave these a work over a week ago and there were no issues, it's just that right now I am away on holiday and have no access to a second monitor.

brownsr commented 8 years ago

Ready for review now I think. I found and fixed a few more bugs as a result of running through a structured test, and have eliminated the allocation/alignment issues in almost all cases. Hopefully I haven't introduced anything I didn't intend in the last few commits. Will re-run the test sometime.

brownsr commented 8 years ago

Have adjusted the panel barriers logic. The errors proved elusive, but hopefully all fixed now (6th April).

Have also suppressed the menu label in a vertical panel. Can be done via the configuration, but this avoids a user thinking that something is wrong, as the presence of a menu label in a vertical panel will disturb the alignment. It's neater this way.

Similarly have forced a fixed time format on a calendar display in a vertical panel. It's all that will fit, and it avoids a user thinking something is wrong or having to find out about time format codes.

Left and right panels will react to margin set up in their css when their size is set. That permits a gap to top and bottom of the screen if required. Similar logic added to horizontal panels for consistency.

BethAr commented 8 years ago

Thanks a lot for working on this!

brownsr commented 8 years ago

A simple video clip showing the features of the PR. It does not show multi-monitor. The sharp eyed may pick up minor bugs mentioned above [the menu icon is slightly misaligned after a panel move. This would be sorted by a cinnamon reset (fixed 10th April). The autohide of the right hand panel will have nuked the corners that should be visible on it with the multi-colour theme]. https://www.dropbox.com/s/41lbyeakmr051wt/cinnamon-20160409c-1.webm?dl=0

ghost commented 8 years ago

@brownsr that is really nice. Can be decide which panel layout commands the desktop? panels

brownsr commented 8 years ago

The style on the left is what I have used, not configurable. The vertical panel will use the whole side in the absence of a horizontal panel though.

ghost commented 8 years ago

Ok, but when you say "The vertical panel will use the whole side in the absence of a horizontal panel though". You say in static mode only, not for example when an horizontal panel is set to auto-hide?

brownsr commented 8 years ago

Static only right now. I thought about making it fully dynamic, but it was in the 'too hard' category at the time. Might return to that at some time, perhaps.

ghost commented 8 years ago

Don't worry, really what you currently do is several time more hard, this are just details. And true, is possible hard to do and is a risk make all at once.

oberon-manjaro commented 8 years ago

@brownsr , that looks really great! How can I test your branch? I have compiled your latest commit, but the panel is not working at all for me. I have even forked your repo, merged it into linuxmint master branch and compiled the 3.1 release including your patches. With that one installed everything works normal, but no vertical panel support... What am I missing?

brownsr commented 8 years ago

@oberon2007 Sorry not to reply earlier, I've only just seen your comment, as I've been doing something else for light relief from this long-running PR. Firstly there appeared to be some conflicts that have been introduced with the latest master commits. Hopefully I have just resolved these, though I appeared to have a minor change in progress which I have not given more than a superficial check, which has also been carried up. Secondly no compilation is needed, as it is all javascript or python code, just copy the changed modules to the requisite places: applet, appletManager, boxpointer, indicatorManager, messageTray, osdWindow, panel, PanelMenu, popupMenu,tooltips to /usr/share/cinnamon/js/ui. cinnamon.css to /usr/share/cinnamon/theme. cs_panel.py to /user/share/cinnamon/cinnamon-settings/modules. applets to the corresponding place in /usr/share/cinnamon/applets. You will need to use sudo. Or you can merge and dpkg-buildpackage I presume, though I've not built that way myself in this particular case.

You won't see any overt changes, but right click on an existing panel will let you set up a new vertical one or move a horizontal one there, and the panel settings will allow a panel to be created on a vertical side. It uses existing functionality, just adds the flexibility of vertical options.

Simple copy script for adapting, I had my development git code under ~/dev/...:

cp ~/dev/Cinnamon/js/ui/applet.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/appletManager.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/boxpointer.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/indicatorManager.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/messageTray.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/osdWindow.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/panel.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/panelMenu.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/popupMenu.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/js/ui/tooltips.js /usr/share/cinnamon/js/ui cp ~/dev/Cinnamon/data/theme/cinnamon.css /usr/share/cinnamon/theme cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/calendar@cinnamon.org/applet.js /usr/share/cinnamon/applets/calendar@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/menu@cinnamon.org/applet.js /usr/share/cinnamon/applets/menu@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/panel-launchers@cinnamon.org/applet.js /usr/share/cinnamon/applets/panel-launchers@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/separator@cinnamon.org/applet.js /usr/share/cinnamon/applets/separator@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/spacer@cinnamon.org/applet.js /usr/share/cinnamon/applets/spacer@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/systray@cinnamon.org/applet.js /usr/share/cinnamon/applets/systray@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/window-list@cinnamon.org/applet.js /usr/share/cinnamon/applets/window-list@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/applets/workspace-switcher@cinnamon.org/applet.js /usr/share/cinnamon/applets/workspace-switcher@cinnamon.org cp ~/dev/Cinnamon/files/usr/share/cinnamon/cinnamon-settings/modules/cs_panel.py /usr/share/cinnamon/cinnamon-settings/modules

oberon-manjaro commented 8 years ago

Thanks a lot, @brownsr, I will experiment a little and report back :smile:

xtemp09 commented 7 years ago

When will it be implemented?

brownsr commented 7 years ago

Intended for cinnamon 3.2 at the moment On 30 Jun 2016 1:50 p.m., "xtemp09" notifications@github.com wrote:

When will it be implemented?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/linuxmint/Cinnamon/pull/4996#issuecomment-229648416, or mute the thread https://github.com/notifications/unsubscribe/AGB5MS6V6crcycGQmDAZw91ketIQQx9lks5qQ7uKgaJpZM4HB-6q .

xtemp09 commented 7 years ago

Thanks, I was about to give up.

brownsr commented 7 years ago

Another (very trivial) PR of mine has been inadvertently caught up in this catch-up to the latest master, that will teach me to make even the smallest PR direct on a master branch ;) I can remove it later if it causes a problem, but I doubt it will.

brownsr commented 7 years ago

@JosephMcc : Hopefully all your comments are acted on now

mtwebster commented 7 years ago

I tested this again - I have a request and a slight bug to report:

The bug - I think? - Adding the stock menu applet to a vertical panel works fine - I like that it just shows the icon, however it doesn't seem to declare itself as a vertical-compatible applet (by implementing getDisplayLayout()) - it just looks at the orientation and acts accordingly.

But, you check here - https://github.com/linuxmint/Cinnamon/pull/4996/files#diff-58e0680d2516e9e58237cf48a393a29bR350 - for what type of applet it is, and a TextIconApplet is an IconApplet, so we end up letting both of those types through (try weather@mockturtl). I think this is easily resolved by adding && !(applet instanceof Applet.TextIconApplet) to that if statement, and making the menu applet actually report that it's a well-behaved vertical panel applet.

brownsr commented 7 years ago

@mtwebster There are associated PRs against mint-y-theme and cinnamon-themes Can you squash it all into one commit when you merge ? There appears to be a recent github facility to do that, from what I read. The gazillion contradictory tutorials on how to squash commits are doing my head in, as none of them seem to describe what to do with all the other people's commits I see, as I have brought it up to date with master to resolve conflicts a number of times. And I don't want to mess things up for you, of course.

Hope to find some time to do the squashing this weekend (13th). If I have problems I will try to merge --squash onto a new branch and will push that instead.