[KRichTextWidget] Add support for headings
ClosedPublic
Actions

Authored by poboiko on Apr 15 2020, 12:09 PM.

Details

Reviewers

mlaurent
ahmadsamir
dfaure

Group Reviewers

Frameworks

Commits

R310:a079e97e905d: [KRichTextWidget] Add support for headings

Summary

This patch adds support of different headings (essentially, HTML h1..h6 tags).
Those might be pretty useful when formatting a rich text (I'm having KJots - note taking app - in mind)

QTextBlockFormat supports headingLevel since Qt 5.12; however, as stated in the documentation,
setHeadingLevel does not adjust style (font size, etc), so we have to take care of it on our own.

It also adds a KSelectAction to choose between different headings.

There are few behavior nuances:

If user presses Enter at the end of heading line, cursor should switch to basic text
If user presses Enter in the middle of the heading, line just breaks into two headings
If user presses Backspace at the beginning of a line after the heading, the line is merged with the previous one (and thus becomes heading itself)
The same with Delete at the end of heading line: the next line should become heading too.

Test Plan

See the video of KRichTextEditor from KXmlGui (I had to add format_heading_level to rc-file).

Diff Detail

Repository

R310 KTextWidgets

Branch

textwidget-heading (branched from master)

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 25278
Build 25296: arc lint + arc unit

poboiko created this revision.Apr 15 2020, 12:09 PM

Restricted Application added a project: Frameworks. · View Herald TranscriptApr 15 2020, 12:09 PM

poboiko requested review of this revision.Apr 15 2020, 12:09 PM

Harbormaster completed remote builds in B25270: Diff 80193.Apr 15 2020, 12:09 PM

poboiko edited the test plan for this revision. (Show Details)Apr 15 2020, 12:10 PM

Can you improve autotest KRichTextEditTest ?

src/widgets/krichtextedit.cpp
346	const int
src/widgets/krichtextedit.h
351	since @5.70
src/widgets/krichtextwidget.cpp
499	const QStringList
src/widgets/krichtextwidget.h
233	@since 5.70

This revision now requires changes to proceed.Apr 15 2020, 1:41 PM

Added a unit-test testing for everything I could come up with
(at least 4 "behavior nuances" from the commit message)

Address other feedback as well (const'ify, @since version)

Harbormaster completed remote builds in B25278: Diff 80202.Apr 15 2020, 2:39 PM

Thanks

This revision is now accepted and ready to land.Apr 15 2020, 7:06 PM

dfaure requested changes to this revision.Apr 15 2020, 8:05 PM

dfaure added inline comments.

src/widgets/krichtextedit.cpp
346	If boundedLevel is 6, the size adjustement will be -1? Does this mean a heading that's smaller than normal text?
375	This seems to duplicate what happened with selectCursor already. Why are two cursors necessary to change the style of one paragraph?

This revision now requires changes to proceed.Apr 15 2020, 8:05 PM

Hey, thanks for this @poboiko I used to have to implement this myself, so this will be very useful in my app, O20.Word. ;)

In D28854#649189, @abstractdevelop wrote:

Hey, thanks for this @poboiko I used to have to implement this myself, so this will be very useful in my app, O20.Word. ;)

Sure, you're welcome! I like doing useful stuff :)

src/widgets/krichtextedit.cpp
346	Yes, that's true. I agree it's a bit confusing, but that's the case with HTML too, which I had in mind (see screenshot, it's Chrome) For markdown, it seems like there are only 5 levels, 5th having the same size as normal text (6th level gets ignored, that's Okular) I guess we probably can follow the Markdown way and bound it with 5, just to avoid confusion.
375	I used `selectCursor` to select a line (block) under cursor and then change the style of the selection with `mergeCharFormat` . `cursor` is for actual users cursor, which also has to change the style, so newly typed text will have the same style and heading level too (I use `mergeBlockCharFormat` for that reason - if I use `mergeCharFormat` here, the cursor will remain big after I press `Enter` after a title. However, it will change its size to the smaller one immediately after I start typing). But I don't want to mess with users selection, that's why I'm keeping both.

mlaurent added inline comments.Apr 18 2020, 1:00 PM

src/widgets/krichtextedit.cpp
346	But if you insert html text from html with title6 how it will work if it just supports h5 ?

I see, thanks for the explanations.

I don't mind 6 levels if that's what HTML does.

This revision is now accepted and ready to land.Apr 18 2020, 1:20 PM

poboiko closed this revision.Apr 18 2020, 3:01 PM

poboiko mentioned this in D28967: [KRichTextEditor] Add support for headings.Apr 18 2020, 10:05 PM

poboiko mentioned this in R263:2f86040cab5b: [KRichTextEditor] Add support for headings.Apr 19 2020, 11:14 AM

poboiko mentioned this in D29300: [KJots] Add support for headings.Apr 30 2020, 3:01 PM

poboiko mentioned this in R573:cfee7c265b02: [KJots] Add support for headings.May 9 2020, 10:05 AM

Revision Contents
Changeset List

			Path	Packages
M			autotests/krichtextedittest.h (1 line)
M			autotests/krichtextedittest.cpp (60 lines)
M			src/widgets/krichtextedit.h (10 lines)
M			src/widgets/krichtextedit.cpp (53 lines)
M			src/widgets/krichtextwidget.h (12 lines)
M			src/widgets/krichtextwidget.cpp (37 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	80193	1d1eb6f		Apr 15 2020, 12:09 PM	★	★
Diff 2	80202	1d1eb6f	Added a unit-test testing for everything I could come up with	Apr 15 2020, 2:38 PM	★	★