New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

U4-11168 : Making the dropdown menu more user friendly within the Document Type icon picker #2555

Closed
wants to merge 32 commits into
base: dev-v7
from

Conversation

Projects
None yet
5 participants
@OwainWilliams
Contributor

OwainWilliams commented Mar 30, 2018

No description provided.

OwainWilliams added some commits Mar 29, 2018

Merge pull request #1 from umbraco/dev-v7
Getting latest changes

@OwainWilliams OwainWilliams changed the title from Temp u4 11168 - Making the dropdown menu more user friendly within the Document Type icon picker to U4-11168 : Making the dropdown menu more user friendly within the Document Type icon picker Mar 31, 2018

@nul800sebastiaan

Thanks @OwainWilliams - I understand the thought behind this but unfortunately it becomes a bit of an unreadable mess. I think it would be way better to have each color represented as a square in front of it's name, like the way Chrome does it for example:

image

Any chance you can look into doing that instead?

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Apr 4, 2018

Hi @nul800sebastiaan
Funny you should say this, I've been looking in to this option because I totally agree it's not the most accessible option. Leave it with me and I'll see what I can do 👍

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Apr 4, 2018

I agree it should use a different approach than changing the text color of the options.

Something like Github label colors:
image

An option could be to use the umbDropdown component, but I am not sure how well it works with many items (scroll).
https://our.umbraco.org/apidocs/ui/#/api/umbraco.directives.directive:umbDropdown

Alternatively it could use a third party component like Selectize https://selectize.github.io/selectize.js/ (or another component (Angular/jQuery/javascript) extending the navtive dropdownlist, which allow to have custom labels like these https://tabler.github.io/tabler/form-elements.html (based on Selectize).

image

Maybe @madsrasmussen has some input on this? 😉 🦄

@madsrasmussen

This comment has been minimized.

Member

madsrasmussen commented Apr 4, 2018

I think it is a great idea to add the colored box next to the label instead of coloring the label itself.

As far as I remember the umbDropdown component should be able to do what you are looking for. You might need to set a max-height on the drop-down element though to allow scrolling.

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Apr 4, 2018

A few other ideas could be to list the defined colors and act like a radiobutton list http://prplv.me/ngjs-color-picker/#/ or some kind of dialog (e.g. by using the umbDropdown component) or a custom directive/component https://bladepop.github.io/colorpicker/

Some other inspiration:
https://www.jqueryscript.net/demo/Flat-HTML5-Palette-Color-Picker-For-jQuery-colorPick-js/
https://cushionapp.com/journal/improved-form-ux

http://daxushequ.com/color-picker/60376804.html
http://daxushequ.com/color-picker/60458738.html

@nul800sebastiaan

This comment has been minimized.

Member

nul800sebastiaan commented Apr 4, 2018

Oh nice! Yes, no need for it to be a dropdown, the color names are not actually used anywhere else anyway.

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Apr 4, 2018

Brilliant - love the ideas. I'll have a look and see what I can come up with over the coming days.

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Apr 15, 2018

Thinking about implementing something like this: https://codepen.io/richiksc/pen/dovpyB
What do you all think?

Alternatively, why even limit the user to the colours they can pick. Just give them a colour picker with an droplet picker e.g. https://codepen.io/Wardee-IDUK/pen/zqyWWg

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Apr 19, 2018

I found these two color/swactch pickers, which are simple, yet pretty awesome.
https://saintplay.github.io/vue-swatches/
https://xiaokaike.github.io/vue-color/

Alternatively there is a swatch picker like this.
https://www.webcomponents.org/element/PolymerElements/paper-swatch-picker

I think it is fine to limit the colors to some predefined colors for consistency.

I like all these three examples.

@madsrasmussen any thoughts on these examples? should we have a "swatch picker" component like this in core? Maybe to be re-useable in prevalue editors and property editors?

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Apr 19, 2018

Nice @bjarnef - I've got some free time next week so I plan to jump back on to this. 👍

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Apr 19, 2018

Found the solution I'm going to try and implement - unless others have better ideas:

http://prplv.me/ngjs-color-picker/#/
image

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented May 6, 2018

Added colour picker options instead of a dropdown list.
image

When the user hovers over a colour, they get a tooltip telling them what colour they are hovering on.

Once a colour is selected, a shadow is placed behind the colour to show that it's selected as well as all the icons changing colour.
image

@abjerner

This comment has been minimized.

Contributor

abjerner commented May 6, 2018

@OwainWilliams I haven't looked code, but your screenshots look awesome 👍

One tiny detail I've noticed is that the rounded corners (border radius) may be a little too much compared to other components in Umbraco.

Umbraco appears to be using a border radius of 3 pixels - eg:

image

image

I'll look forward to seeing this in the core ;)

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented May 6, 2018

@OwainWilliams I think it would be great to create a array of objects for the colors and then use ng-repeat and ng-model
Instead of button element you could use a radiobutton list with hidden radiobuttons and the color selections are the labels as in my package Color Palettes or you could use another elements like span or div with ng-repeat as in this example:
https://codepen.io/ksymeon/pen/qzAjy

It would also be great to create it as a directive, so it could be re-used e.g. as a property editor or a prevalue editor editor, incl. in grid editor settings/styles config.

Changing order of colours
Changed the order of the colours and also took the drop shadow off the buttons. Put a grey border around the active button instead.

OwainWilliams and others added some commits May 8, 2018

Fixing pull issues!
Hit problems with pull requests overwriting the original changes!
Removing files
removing files that shouldn't be part of my PR and updating my gitignore file.
Update gitignore
Missed a file
Update gitignore
Missed a file
Remove unused file
removing ngjs-color-picker.js as it's now not used.
@nul800sebastiaan

This comment has been minimized.

Member

nul800sebastiaan commented Jun 4, 2018

Note: when we get to this PR, we'll also incorporate the sorting of the colors as requested in #2657

Merge pull request #2 from bjarnef/temp-color-swatches-directive
WIP - Add basic color swatches directive
@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jun 22, 2018

Any suggestions on how to fix the conflict?

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Jun 22, 2018

@OwainWilliams if you merge the branches in e.g. SourceTree you can choose something like "Use Mine", "Use Theirs" or "Merge using external" tool. I use Beyond Compare as a compare/merge tool, but WinMerge should work as well and is free to use.

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jun 22, 2018

@bjarnef It's strange because my local repo says there are no conflicts.

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Jun 22, 2018

@OwainWilliams it seems from what I can see in the changes it removes this file.
src/Umbraco.Web.UI/Umbraco.Web.UI.csproj which was removed in this commit 6db0b10

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jun 22, 2018

So, maybe take this file out of the ignore file?

@bjarnef

This comment has been minimized.

Contributor

bjarnef commented Jun 22, 2018

Yes, I think.
I would be easiest to review if the PR only has changes relevant to this specific change.
https://github.com/umbraco/Umbraco-CMS/pull/2555/files

This isn't necessary to add for this PR:
image

and I guess you could revert this change or merge this part from upstream/dev-v7

image

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jun 25, 2018

Agggh! This is totally bust now. Going to need to look at reverting everything I think to a previous verison.

@nul800sebastiaan

This comment has been minimized.

Member

nul800sebastiaan commented Jun 25, 2018

@OwainWilliams Hold on, relax, we can look at it. :)

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jun 25, 2018

Cheers @nul800sebastiaan - I'll wait but if you need me to do anything, let me know.

@nul800sebastiaan

This comment has been minimized.

Member

nul800sebastiaan commented Jul 13, 2018

@OwainWilliams Alright mate, had a bit of a look and indeed it's a bit of a mess! 🙈

Not to worry though. Let's just start over. The easiest thing for you to do now is to create a fresh clone of your fork in a different directory than your current clone. You'll end up with dev-v7 as default.

Make sure to fetch the changes from the main repository, there's a few commands listed here that will help you get the latest changed from dev-v7.

Then you can create a new branch from dev-v7 and call it something different than your current branch, temp-U4-11168-2 maybe, with the -2 suffix.
Finally, start applying the same changes you made in https://github.com/umbraco/Umbraco-CMS/pull/2555/files?w=1

I see that in the process, you've deleted all kinds of files and updated .gitignore - there's no need for that as far as I can tell. If you see files pop up during the commit phase that shouldn't be there, then just revert them or don't commit them. I'm not sure why you deleted the Umbraco.Web.UI.csproj file, that's definitely not going to work. 😉

Note that I've recently merged a PR that affects your changes as well: #2712
It would be good if you could have a look if that still works alongside your changes, it looks like it should.

Another thing I noticed in your changes is that you've collapsed some of the less styles. For readability, make sure to just keep them as they were.

image

We process and minify less files during the grunt build already. 😄

I'll close this one for now. I think starting fresh with all the knowledge we have from the discussions above is going to make it very easy to implement this feature easily now by just copy/pasting the changes over!

Great work so far Owain, I tested out the changes earlier and they work wonderfully. So once you can submit a new PR, we'll be able to merge them in pretty quickly. Good times! 🎉

@nul800sebastiaan

This comment has been minimized.

Member

nul800sebastiaan commented Jul 13, 2018

Oh and it would be good of you re-order the colors a little, as suggested in #2657 !

@OwainWilliams

This comment has been minimized.

Contributor

OwainWilliams commented Jul 13, 2018

Cheers for the feedback! I'll see if I can get on to this at lunch time and over the weekend. All makes sense. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment