Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Fix: The value of the AttributeCommand is taken from the first allowed node #72

Merged
merged 5 commits into from Jul 2, 2018

Conversation

oskarwrobel
Copy link
Contributor

@oskarwrobel oskarwrobel commented Jun 29, 2018

Suggested merge commit message (convention)

Fix: The value of the AttributeCommand is taken from the first allowed node. Closes ckeditor/ckeditor5#5548.


Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

@oskarwrobel oskarwrobel requested a review from pjasiun June 29, 2018 14:26
@coveralls
Copy link

coveralls commented Jun 29, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling b782e9a on t/56 into e19c90a on master.

const ranges = Array.from( selection.getRanges() );

if ( selection.isBackward ) {
ranges.reverse();
Copy link

Choose a reason for hiding this comment

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

Are you sure it is needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Checking range direction is removed.

@@ -41,7 +41,7 @@ export default class AttributeCommand extends Command {
* Flag indicating whether the command is active. The command is active when the
* {@link module:engine/model/selection~Selection#hasAttribute selection has the attribute} which means that:
*
* * If the selection is not empty – That it starts in a text (or another node) which has the attribute set.
* * If the selection is not empty – That the first node in the selection that allows the attribute has the attribute set.
Copy link

Choose a reason for hiding this comment

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

the selection that allows the attribute has the attribute set

Something is wrong in this sentence.

* Checks the attribute value of the first node in the selection that allows the attribute.
* For the collapsed selection returns the selection attribute.
*
* **Note** Selection direction is taken into consideration.
Copy link

Choose a reason for hiding this comment

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

No, it's not anymore ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right :)

@pjasiun pjasiun merged commit 64a0dbc into master Jul 2, 2018
@pjasiun pjasiun deleted the t/56 branch July 2, 2018 14:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants