fix(NcRichText): start heading from h4 #7748
Conversation
ShGKme
requested review from
a team,
DorraJaouad,
JuliaKirschenheuter and
susnux
ShGKme
added
bug
ShGKme
force-pushed
the
fix/NcRichText--h4
branch
from
f49b29f to
aaa836a
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7748 +/- ##
=======================================
Coverage 43.38% 43.39%
=======================================
Files 277 277
Lines 17990 17996 +6
Branches 1009 1010 +1
=======================================
+ Hits 7805 7809 +4
- Misses 10134 10136 +2
Partials 51 51 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
jancborchardt
left a comment
jancborchardt
left a comment
There was a problem hiding this comment.
Interesting approach – I guess it is indeed still semantic given as you say h1, h2 and h3 are used on the page. But not sure we could always guarantee no heading level is skipped?
|
/backport to stable8 |
Signed-off-by: Grigorii K. Shartsev <[email protected]>
ShGKme
force-pushed
the
fix/NcRichText--h4
branch
from
aaa836a to
abdc336
Compare
|
Adjusted tests |
We cannot. But skipping isn't a problem if it doesn't break the structure. Users can also start with We can also use And the worst idea I have — actually query for the heading to start from the next level. But I'd avoid it. |
| // Using them for user content leads to accessibility issues | ||
| // Levelling down headings to start from <h4> | ||
| if (['h1', 'h2', 'h3', 'h4', 'h5', 'h6'].includes(String(type))) { | ||
| type = `h${Math.min(+String(type)[1] + 3, 6)}` |
There was a problem hiding this comment.
Is this always valid? What if the component is used right under a h1?
Maybe we need some magic like $el.closest and then check whats the closest headline?
There was a problem hiding this comment.
What if the component is used right under a h1?
Answered in the comment above
There was a problem hiding this comment.
can we maybe apply some classes to visually differentiate #### heading from ###### heading?
There was a problem hiding this comment.
IMO, distinguish ##### from ###### must be already an edge case. It should mean that 4 heading levels were already used, and you still need 2 more levels. It's hard to imagine for me even in a dissertation, and this component is just for a comment/message.
And if users use h5-h6 without upper levels, then there is probably something wrong. For example, they may do it because h1-h3 are way too large. This is why I'm keeping them smaller here.
@kra-mo Maybe you have any opinion here? Should we have different styles for all h1..h6 and if we do, which style (keeping h1 small enough for a comment/message)?
There was a problem hiding this comment.
@kra-mo Maybe you have any opinion here?
Personally, I don't see the need to differentiate visually. Some apps make headings even smaller than body text or use all-caps to indicate the levels, but I think both are suboptimal.
kra-mo
left a comment
kra-mo
left a comment
There was a problem hiding this comment.
Nice, this will be a great improvement :)
|
Merging with 2 designers approving the design with equal low-level heading styles. |
|
The backport to # Switch to the target branch and update it
git checkout stable8
git pull origin stable8
# Create the new backport branch
git checkout -b backport/7748/stable8
# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick abdc336d
# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/7748/stable8Error: Failed to check for changes with origin/stable8: No changes found in backport branch Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports. |
☑️ Resolves
🖼️ Screenshots
🏁 Checklist
stable8for maintained Vue 2 version or not applicable