mail icon indicating copy to clipboard operation
mail copied to clipboard

Change the 'show image' styling

Open GretaD opened this issue 3 years ago β€’ 7 comments

this is how i tried to make it work with the state of actions we have now. But i agree with Marco's comment here: https://github.com/nextcloud/mail/issues/6122#issuecomment-1202480324

before Screenshot from 2022-08-02 15-35-35 Screenshot from 2022-08-02 15-35-22

after showimage-afeter showimage_after

  • [ ] Requires https://github.com/nextcloud/nextcloud-vue/pull/2911

GretaD avatar Aug 02 '22 13:08 GretaD

@jancborchardt and @nimishavijay please have a look at this solution and if you're ok with it

GretaD avatar Aug 02 '22 13:08 GretaD

  • [ ] "Show image" β†’ "Show images"

  • [ ] The eye icon is not necessary, as per mockup

  • [ ] In the menu, the icons are all the same, so either we should go with no icons or:

    • Temporarily: https://fonts.google.com/icons?selected=Material%20Icons%3Aimage%3A
    • Show from this email: https://fonts.google.com/icons?selected=Material%20Icons%3Amail%3A
    • Show from this domain: https://fonts.google.com/icons?selected=Material%20Icons%3Adomain%3A

Regarding the comment from @marcoambrosini, either is fine with me. Indeed using the tertiary button component would be more accurate since it’s not a link which goes somewhere.

all the point can be delivered but the icon one. The action must have an icon, it cannot be removed unfortunately, sorry should have commented that before. If we wait for the vue changes, this will be delivered a bit late, if we move on with my solution, it will look like this(plus your comments, but with an icon)

GretaD avatar Aug 02 '22 16:08 GretaD

@GretaD cool, and fine for now with the icon too, but then also use the "image" filetype icon instead of "eye".

jancborchardt avatar Aug 02 '22 18:08 jancborchardt

We can definitely add the option not to have an icon and have the menus triggered by tex-tonly buttons. I think it totally makes sense.

marcoambrosini avatar Aug 03 '22 06:08 marcoambrosini

...and that will automatically be the case when https://github.com/nextcloud/nextcloud-vue/issues/2768 happens

marcoambrosini avatar Aug 03 '22 06:08 marcoambrosini

We can definitely add the option not to have an icon and have the menus triggered by tex-tonly buttons. I think it totally makes sense.

i know that, but when it is going to be released? :)

GretaD avatar Aug 03 '22 07:08 GretaD

pr is there https://github.com/nextcloud/nextcloud-vue/pull/2911#issuecomment-1207891395 Let's wait for more reviewers since it's a big change. Also you can review @GretaD :)

Once that's we can do a minor release

marcoambrosini avatar Aug 08 '22 09:08 marcoambrosini

How is this unblocked when it depends on https://github.com/nextcloud/nextcloud-vue/pull/2911 and this app still uses v5?

ChristophWurst avatar Aug 16 '22 14:08 ChristophWurst

How is this unblocked when it depends on nextcloud/nextcloud-vue#2911 and this app still uses v5?

ah sorry didnt see you added the component on desc. I am not using that component, that was not related to actions without icons, what Marco meant(double checked with him) is that after that is in, we can edit the actions on vue again and add a slot with no icons, but as you i thought that that was supposed to be done on the same pr,

What i did here was hiding the icon with display none. I dont have time to spend on the component now, so we can either do this or wait till we have time.

GretaD avatar Aug 16 '22 14:08 GretaD

not ready to merge

GretaD avatar Aug 17 '22 14:08 GretaD

not ready to merge

ready now, please reviewers test it

GretaD avatar Aug 17 '22 14:08 GretaD