Use large border-radius in Action Button #2713
Conversation
Signed-off-by: greta <gretadoci@gmail.com>
GretaD
added
enhancement
GretaD
requested review from
marcoambrosini,
raimund-schluessler,
st3iny,
kesselb and
ChristophWurst
GretaD
added
2. developing
|
Ready for review or not? |
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
|
@GretaD I had a bit of time and worked on your PR. I hope, you don't mind. If you don't like my changes, just discard them. The result now looks like this: The values of the border radius might need some tweaking, though. Given that we should respect OuterRadius = InnerRadius + Padding, the OuterRadius is 14 px at the moment, which might be a bit large. If we want it smaller, we would need to reduce either padding and/or InnerRadius. |
| @@ -170,6 +170,7 @@ export default { | |||
| :placement="placement" | |||
| :boundaries-element="boundariesElement" | |||
| :container="container" | |||
| popover-base-class="action-item__popover" | |||
There was a problem hiding this comment.
Necessary, so we can only style popovers from Actions.
| @@ -77,6 +77,13 @@ export default { | |||
| VPopover, | |||
| }, | |||
|
|
|||
| props: { | |||
| popoverBaseClass: { | |||
There was a problem hiding this comment.
Needs some documentation.
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
I adjusted the border radii. This looks like this now: This is closer to what @nimishavijay had in mind in nextcloud/mail#6200 (comment), I guess. |
No, sorry, my first approach was not the perfect one and raimund had a good point on fixing everything here on vue instead of getting sever involved |
No problem, i was working on this yesterday and i did smth similar but the row jumped a bit on hover, thats why i didnt push. |
I tested it with Tasks and it worked fine from my POV. But a bit more testing wouldn't hurt, I guess. It affects a widely used component, though. |
|
works good with mail too |
There was a problem hiding this comment.
I tested it on calendar and it worked well. It looks very good too.
I couldn't test on mail because the app won't load and I get the following error in the console:
TypeError: __webpack_modules__[moduleId] is not a function
__webpack_require__ isMobile.js:107
But I don't think that's related.
EDIT: Nevermind, I did npm ci and npm run dev again and it worked.
|
Shall we tag this as breaking? I don't know how high impact design changes are handled here |
ref: nextcloud/mail#6200 (comment)