fix(NcSettingSection): update design and remove limitWidth option
#5514
Conversation
marcoambrosini
force-pushed
the
bugfix/remove-custom-heading-styles
branch
from
cd37441
to
c33516f
Compare
marcoambrosini
changed the title
marcoambrosini
force-pushed
the
bugfix/remove-custom-heading-styles
branch
from
c33516f
to
c39bde5
Compare
marcoambrosini
added
enhancement
| /** | ||
| * Limit the width of the setting's content | ||
| * | ||
| * By default only the name and description have a limit, use this | ||
| * property to also apply this to the rest of the content. | ||
| */ | ||
| limitWidth: { | ||
| type: Boolean, | ||
| default: true, | ||
| }, |
There was a problem hiding this comment.
Technically this is a "small" breaking change, because it changes the component's API. It doesn't really break the page, but it results in invalid HTML attribute limit-width in DOM.
We can mark this prop as @deprecated here and remove in backport to next
There was a problem hiding this comment.
doesn't it break the page for people who used full width sections?
There was a problem hiding this comment.
Appearance and Accessibility is a bit broken
There was a problem hiding this comment.
there's another pr here where we're tackling all those regressions
nextcloud/server#44960
There was a problem hiding this comment.
@marcoambrosini @ShGKme I assume for appearance and accessibility we can just make it single-column instead of putting 2 themes next to each other?
There was a problem hiding this comment.
We can, but I personally like 2 column design instead of long page.
The same problem with user migration app and privacy.
There was a problem hiding this comment.
It can be 2 columns and text below the image. But these will be addressed in a separate pr in server. Let's get this in :)
There was a problem hiding this comment.
as far as I can see, the prop was not actively used: https://github.com/search?q=org%3Anextcloud-libraries%20limit-width&type=code
There was a problem hiding this comment.
as far as I can see, the prop was not actively used: https://github.com/search?q=org%3Anextcloud-libraries%20limit-width&type=code
You need to look in the nextcloud org, as it is for apps:
https://github.com/search?q=org%3Anextcloud%20limit-width&type=code
Signed-off-by: Marco Ambrosini <marcoambrosini@proton.me>
marcoambrosini
force-pushed
the
bugfix/remove-custom-heading-styles
branch
from
80b8329
to
fea8d49
Compare
marcoambrosini
dismissed
jancborchardt’s stale review
This will be tackled separately in server pr
|
Can we please get this in? |
ShGKme
added
bug
ShGKme
changed the title
limitWidth option
|
Notes: this is not a new feature (no new components/props/events/slots), but a change behavior of the component so it should have Both labels and PR title are used for automatic |
|
So we spin up a new major now? Because this is breaking apps and server -> We can not update |
|
We can do for next version, so backport |
|
/backport to next |
fix: Revert #5514 and only force width limit on Nextcloud 30+
Remove custom heading, remove full width option and other minor fixes