Skip to content

Conversation

@GretaD
Copy link
Contributor

@GretaD GretaD commented Jul 2, 2025

The problem:
NcActionText has a br to split action name from action_long text, which breaks our quota and quota text on our action menu as seen below on the screenshot.
NcActionButton has a section for long text that doesnt break: https://github.com/nextcloud-libraries/nextcloud-vue/blob/ae1b3c2ef908985a4052ac03d9a6310593030545/src/components/NcActionButton/NcActionButton.vue#L346-L364

so i used that to fix our problem. Another solution would be if we remove the br and add description prop, but im not sure if that works on every scenario.

before
Screenshot from 2025-07-02 12-35-21
after
Screenshot from 2025-07-02 12-35-41

Signed-off-by: greta <[email protected]>
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works visually until you hover, because buttons have a pointer cursor. This wrongly makes the element appear as if it were clickable.

Can you share more insights about the impact of the br in other usages of the text action? Is the additional space desired everywhere but Mail, or should we fix all usages vi an upstream fix?

Also, the fix only covers the accounts actions. Mailbox actions still have the additional space. Please fix both in one go.

@ChristophWurst
Copy link
Member

Another solution would be if we remove the br and add description prop, but im not sure if that works on every scenario.

What would the description prop do?

@GretaD
Copy link
Contributor Author

GretaD commented Jul 2, 2025

Can you share more insights about the impact of the br in other usages of the text action? Is the additional space desired everywhere but Mail, or should we fix all usages vi an upstream fix?

I didnt find any other usage of having a name with a long text, but i feel like im missing something because there is this comment on the ncactiontext component: <!-- white space is shown on longtext, so we can't put {{ text }} on a new line for code readability -->
So, wondering why it was there in the first place, and what can it break if we remove it? So, while wondering, i saw the ncactionbutton change and went with that :).

As i noted on in the ticket, we can also add a desc prop and see how that fits our scenario.

@ChristophWurst
Copy link
Member

As i noted on in the ticket, we can also add a desc prop and see how that fits our scenario.

There is no such prop on https://nextcloud-vue-components.netlify.app/#/Components/NcActions?id=ncactiontext

@GretaD
Copy link
Contributor Author

GretaD commented Jul 2, 2025

As i noted on in the ticket, we can also add a desc prop and see how that fits our scenario.

There is no such prop on https://nextcloud-vue-components.netlify.app/#/Components/NcActions?id=ncactiontext

i know, my proposal is to add a (new) prop.

@ChristophWurst
Copy link
Member

I didnt find any other usage of having a name with a long text

Indeed not a lot of those usages.

The usages with name+default slot across our orga are

In other words just Talk and Mail. Is Talk affected by the same design bug?

@ChristophWurst
Copy link
Member

i know, my proposal is to add a (new) prop.

Got it. What will the descr prop do?

@GretaD
Copy link
Contributor Author

GretaD commented Jul 2, 2025

In other words just Talk and Mail. Is Talk affected by the same design bug?

Its broken for Talk too.
Screenshot from 2025-07-02 13-50-44

I found this pr from Maksime and wanted to propose the same for ncactiontext: nextcloud-libraries/nextcloud-vue#6932

@GretaD
Copy link
Contributor Author

GretaD commented Jul 2, 2025

I guess now this is obvious that needs to be changed upstream. So i will do that

@ChristophWurst
Copy link
Member

Yep sounds like the best approach if there is not a single use where the current style looks good :)

@ChristophWurst
Copy link
Member

I found this pr from Maksime and wanted to propose the same for ncactiontext: nextcloud-libraries/nextcloud-vue#6932

That makes sense if you want to add more text to an existing action. In our case the entry is fully informational and standalone.

@GretaD
Copy link
Contributor Author

GretaD commented Jul 2, 2025

closing. fixing upstream nextcloud-libraries/nextcloud-vue#7116

@GretaD GretaD closed this Jul 2, 2025
@ChristophWurst ChristophWurst deleted the fix/quota-spacing branch July 2, 2025 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants