Host avatar and color chooser.

[mvollmer]

Should be feature complete. Coming later:

• More clever way of storing the dashboard config without cockpitd.

[andreasn]

Cool stuff. Some remarks:

• Missing icons obviously. We can get these from patternfly and the Fontawesome font
• host-edit-avatar id needs cursor: pointer on hover
• host-edit-color-popover .color-cell needs border on hover
• default image is too small and needs a higher res image

Will fix these in a pull request against your branch

[mvollmer]

• default image is too small and needs a higher res image

We can use server-large.png, I think. Because of the way the dialog works, we will never accidentally save it as the real avatar.

[andreasn]

Here are the fixes mentioned above:
https://github.com/andreasn/cockpit/tree/multi-dash-edit-style

[andreasn]

Images that don't have the same height as width becomes stretched it seems.

[andreasn]

I think the best in those cases is to include a neutral (white?) background that can become part of the image, but create a square cropping area that is the size of the shorter edges.

[andreasn]

I also just found this very embarrassing visual fuckup. Will provide a fix.

[andreasn]

fixed the visual error in this commit https://github.com/andreasn/cockpit/tree/multi-dash-edit-style2

[mvollmer]

Images that don't have the same height as width becomes stretched it seems.

Right, good catch. The old code has a fix for this, I think.

[mvollmer]

Updated with new behavior:

• Color and avatar are now stored with the dashboard configuration, not on the individual machines.
• The pretty name of a machine is cached in the dashboard configuration.
• Set edit dialog can also change the pretty name of a machine, but only when we are connected to it, obviously.

[mvollmer]

@andreasn, should the edit and delete button disappear after clicking on one of them?

[mvollmer]

I think the best in those cases is to include a neutral (white?) background

Transparent?

[mvollmer]

but create a square cropping area that is [initially] the size of the shorter edges.

Hmm, I would propose to set the initial cropping area to be as big as possible. That way you can just press OK to use the whole image. It will have some transparent borders, but that's OK.

[mvollmer]

Ready for more review. I'll add some tests.

[andreasn]

Making a square that is by default covering the whole image ensures that even this picture below works, especially since we're displaying them at quite small sizes. The white borders would dominate otherwise.

This is also more or less what Facebook, Google+ and Twitter does.

[andreasn]

Transparent?

I would say white, otherwise the background will bleed through on hover and stuff, and that would look odd.

[andreasn]

@andreasn, should the edit and delete button disappear after clicking on one of them?

As mentioned on IRC, yes!

[stefwalter]

@andreasn does it make sense to work through the above issues in separate bugs? If not, I don't mind waiting for fixes before we merge this.

[mvollmer]

Rebased and added a FIXUP for the lingering popover.

[andreasn]

None of the icons in Server pane of dashboard are not centered within their buttons.

I can't replicate this issue. Can you post a screenshot?

The rest of the issues are fixed here: https://github.com/andreasn/cockpit/tree/hosts-edit-style-polish

[stefwalter]

I can't replicate this issue. Can you post a screenshot?

All of the 4 buttons in the 'Servers' area are not centered, but it is most apparent with the symetrical buttons like [+] and [x].

[andreasn]

Fixed on the branch mentioned above. Should be ready.

[stefwalter]

Some remaining issues:

• The [Set] and [Cancel] buttons are still different sizes
• The width of the hover box around the avatar does not match the width. It is wider by several pixels.
• The hover color for the avatar should match the patternfly hover color for the server name text field, or match its gray non-hover state. We figured that on the color field the hover wasn't visible, perhaps having black on the color field is an okay work around. But having a black hover for the avatar box is jarring in comparison to the hover over the server name text field.

[andreasn]

The [Set] and [Cancel] buttons are still different sizes

I realize you must be talking about the height. I read your comment as in that they were different width (buttons always are) and that the Set should be bigger to not be dominated by Cancel next to it. The height issue is indeed a regression introduced globally now. I've found the error and have a fix, but need to test it a bit more.

[andreasn]

The width of the hover box around the avatar does not match the width. It is wider by several pixels.

After failing miserably at fixing this with border, I realized box-shadow would do the trick. This also simplifies the css!

[andreasn]

The hover color for the avatar should match the patternfly hover color for the server name text field

These and the two other issues should now be fixed on the same branch. The scariest part of the pull request is that all element with the class btn will now have a min-height and a min-width of 28px. As far as I can see this have not affected any other part of Cockpit in a negative way.

[stefwalter]

One last issue. I cannot click away from the color picker. I have to choose a color. The color picker is a popover and it should go away when I click somewhere else. Try this:

• Open 'Change Server Appearance' dialog
• Click color
• Now without clicking a color, click the avatar, and choose a new image
• The color picker is still there obscuring the image.

[mvollmer]

Squashed and rebased.

[mvollmer]

One last issue. I cannot click away from the color picker. I have to choose a color.

You can click on the big colored button again and it will close.

I have changed it to use the dropdown plugin and now it behaves like a menu.

[stefwalter]

Color pop over does not go away even when i choose a color now. Did you forget to push something?

[mvollmer]

Color pop over does not go away even when i choose a color now.

This has been broken by jquery.js being loaded twice, once from shell.html and a second time via our AMD loader.

[stefwalter]

jQuery bug fixed in #1526