Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow passing config.ignoreStyle to widgetTools.addTests as function type #3230

Merged
merged 6 commits into from
Jul 9, 2019
Merged

Allow passing config.ignoreStyle to widgetTools.addTests as function type #3230

merged 6 commits into from
Jul 9, 2019

Conversation

engineering-this
Copy link
Contributor

What is the purpose of this pull request?

Task

Does your PR contain necessary tests?

All patches which change the editor code must include tests. You can always read more
on PR testing,
how to set the testing environment and
how to create tests
in the official CKEditor documentation.

This PR contains

  • Unit tests
  • Manual tests

What is the proposed changelog entry for this pull request?

none

What changes did you make?

config.ignoreStyle passed to widgetTools.addTests can now be function type. Also updated uses in mathjax-mock test.

Closes #3173

@jacekbogdanski jacekbogdanski self-requested a review July 5, 2019 12:15
@jacekbogdanski jacekbogdanski self-assigned this Jul 5, 2019
jacekbogdanski
jacekbogdanski requested changes Jul 5, 2019
View reviewed changes
Copy link
Member

@jacekbogdanski jacekbogdanski left a comment

Choose a reason for hiding this comment

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

Updated mathjax tests are failing. You only updated only one place, where config.ignoreStyle is passed in a couple of places during the execution of the test suite. Passing delegate results in truthy value where it shouldn't.

tests/plugins/mathjax/mathjax-mock.js Outdated Show resolved Hide resolved
tests/plugins/widget/_helpers/tools.js Outdated Show resolved Hide resolved
tests/plugins/widget/_helpers/tools.js Outdated Show resolved Hide resolved
tests/plugins/widget/_helpers/tools.js Outdated Show resolved Hide resolved
@engineering-this engineering-this self-assigned this Jul 8, 2019
jacekbogdanski
jacekbogdanski requested changes Jul 9, 2019
View reviewed changes
Copy link
Member

@jacekbogdanski jacekbogdanski left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jacekbogdanski jacekbogdanski self-requested a review July 9, 2019 11:29
jacekbogdanski
jacekbogdanski approved these changes Jul 9, 2019
View reviewed changes
Copy link
Member

@jacekbogdanski jacekbogdanski left a comment

Choose a reason for hiding this comment

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

LGTM 👍 😄

@f1ames f1ames self-requested a review July 9, 2019 12:04
@f1ames f1ames self-assigned this Jul 9, 2019
@jacekbogdanski jacekbogdanski changed the base branch from master to major July 9, 2019 12:20
@jacekbogdanski
Copy link
Member

Changed upstream into major and rebased.

@f1ames
Copy link
Contributor

f1ames commented Jul 9, 2019

Fixed one commit message.

f1ames
f1ames approved these changes Jul 9, 2019
View reviewed changes
Copy link
Contributor

@f1ames f1ames left a comment

Choose a reason for hiding this comment

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

👍

@f1ames
Copy link
Contributor

f1ames commented Jul 9, 2019

Rebased on the latest major.

@f1ames f1ames merged commit 0e850e0 into major Jul 9, 2019
@CKEditorBot CKEditorBot deleted the t/3173 branch July 9, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jacekbogdanski jacekbogdanski jacekbogdanski approved these changes

@f1ames f1ames f1ames approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve widgetTools.addTests to evaulate ignoreStyle option during runtime
3 participants
@engineering-this @jacekbogdanski @f1ames