Opened 9 years ago

Closed 9 years ago

Last modified 5 years ago

#10237 closed enhancement (fixed)

[patch][ccla] Format can't be removed/reset via dropdown

Reported by: rsccag Owned by: Jared Jurkiewicz
Priority: high Milestone: 1.5
Component: Editor Version: 1.3.2
Keywords: editor format dropdown Cc:
Blocked By: Blocking:

Description

After a format has been applied to selected text via the dropdown menu, it is not possible to remove the format again. Only another format can be applied. This is not consistent since entered text has no initial format. There should be an entry named 'none' or 'remove format'.

Attachments (3)

dojobuga.jpg (30.2 KB) - added by rsccag 9 years ago.
Apply format to created text
dojobugb.jpg (30.0 KB) - added by rsccag 9 years ago.
Format can only be changed the is no going back
Unset_format.patch (10.0 KB) - added by Jared Jurkiewicz 9 years ago.
Patch to fix the issue. Adds new function to selection code. This can be reused (I do similar in the NormalizeIndentOutdent?? code), so with that add, I can code-reduce that dojox plugin as well.

Download all attachments as: .zip

Change History (21)

Changed 9 years ago by rsccag

Attachment: dojobuga.jpg added

Apply format to created text

Changed 9 years ago by rsccag

Attachment: dojobugb.jpg added

Format can only be changed the is no going back

comment:1 Changed 9 years ago by Jared Jurkiewicz

Resolution: invalid
Status: newclosed

There is a remove format command supported by the editor: http://docs.dojocampus.org/dijit/Editor#builtin-commands

'removeFormat'

Enable it and see if it works the way you expect. Reopen if it does not.

comment:2 Changed 9 years ago by Jared Jurkiewicz

Resolution: invalid
Status: closedreopened

Hm, does not look like it does. It will remove bold/italic, etc, but not the actual format. removeFormat needs to be augmented.

comment:3 Changed 9 years ago by Jared Jurkiewicz

Owner: set to Jared Jurkiewicz
Status: reopenednew

comment:4 Changed 9 years ago by Jared Jurkiewicz

I've prototype-implemented this. The fix is more complex than I would like, but there is no native 'unformatblock' type command. So I have to manually do it via selection traversal and manipulation. Whee. Still needs unit tests, but the basics work.

Changed 9 years ago by Jared Jurkiewicz

Attachment: Unset_format.patch added

Patch to fix the issue. Adds new function to selection code. This can be reused (I do similar in the NormalizeIndentOutdent?? code), so with that add, I can code-reduce that dojox plugin as well.

comment:5 Changed 9 years ago by Jared Jurkiewicz

Status: newassigned

comment:6 Changed 9 years ago by Jared Jurkiewicz

Summary: Format can't be removed/reset via dropdown[patch][ccla] Format can't be removed/reset via dropdown

comment:7 Changed 9 years ago by Jared Jurkiewicz

(In [21047]) Updating FontChoice? to have a 'none' setting along with helper inSelection function many plugins need. refs #10237

comment:8 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: assignedclosed

(In [21048]) Use selection.js inSelection. code reduction. fixes #10237

comment:9 Changed 9 years ago by bill

Resolution: fixed
Status: closedreopened

Great, thanks for adding this. Please add automated tests for this new feature though.

comment:10 Changed 9 years ago by Jared Jurkiewicz

Working on a test for this. Should be done in a day (or less if my local network behaves!)

comment:11 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed

(In [21111]) Adding in tests for FontChoice? remove format. fixes #10237

comment:12 Changed 9 years ago by Jared Jurkiewicz

(In [21296]) Fixing an issue on IE with format removal, ID identifies DIV as P in queryCommandValue, which causes state issues, so have to manually check it to ensure it really is a P or not. refs #10237

comment:13 in reply to:  12 Changed 9 years ago by davidmark

Replying to jaredj:

(In [21296]) Fixing an issue on IE with format removal, ID identifies DIV as P in queryCommandValue, which causes state issues, so have to manually check it to ensure it really is a P or not. refs #10237

Re-opening. Adding a browser sniff is just buying more trouble. That's why there is a seemingly endless stream of adjustments for this stuff.

comment:14 in reply to:  12 Changed 9 years ago by davidmark

Resolution: fixed
Status: closedreopened

Replying to jaredj:

(In [21296]) Fixing an issue on IE with format removal, ID identifies DIV as P in queryCommandValue, which causes state issues, so have to manually check it to ensure it really is a P or not. refs #10237

comment:15 Changed 9 years ago by Adam Peller

feel free to suggest an alternative, otherwise I suspect this will be closed

comment:16 Changed 9 years ago by Jared Jurkiewicz

Exactly. If you have a solution that doesn't require a sniff, I'm all for it. Otherwise I'm just going to close this.

comment:17 Changed 9 years ago by Jared Jurkiewicz

Resolution: fixed
Status: reopenedclosed

(In [21336]) Just always check if the value is P, since IE returns incorrectly for div. Avoids browser check to just always check. fixes #10237

comment:18 Changed 5 years ago by bill

Jared, is there any reason you didn't clear the block formatting via a simple?

editor.execCommand(command, "DIV");

With the current solution, it collapses multiple blocks into a single block, i.e.

<h1>heading</h1>
<h2>sub - heading</h2>

becomes just

heading sub - heading

I would have expected this:

<div>heading</div>
<div>sub - heading</div>

That is also (visually) what Microsoft Word does when you select the "Clear formatting" choice.

I do realize this is complicated by the fact that EnterKeyHandling? can run in three modes (BR, DIV, P), or not be loaded at all. Still, using

editor.execCommand(command, "DIV");

seems like an improvement over the current behavior, and it gets rid of 100 lines of code.

Note: Another test is to run test_FontChoice.html, and clear all the formatting. Neither solution clears the lists.

Note: See TracTickets for help on using tickets.