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

Paste from Office support for Safari #16

Merged
merged 25 commits into from
Nov 5, 2018
Merged

Paste from Office support for Safari #16

merged 25 commits into from
Nov 5, 2018

Conversation

f1ames
Copy link
Contributor

@f1ames f1ames commented Sep 27, 2018

Suggested merge commit message (convention)

Other: Support for pasting flat list and spacing handling for pasted content in Safari. Closes ckeditor/ckeditor5#2481. Closes ckeditor/ckeditor5#2480. Closes ckeditor/ckeditor5#2511.


Additional information

Bringing support for Safari required few things:

In the context of pasted content it was adapting function for recognizing Word content and handling some spacing issues.

The bigger change was in the way how tests are run. As Safari produces different input when content is pasted (different than Chrome, Firefox and Edge), the mechanism for switching fixtures depending on a browser was needed. Eventually, we have decided that all fixtures can be tested in each browser (thus simplifying the code by skipping browser detection and eliminating the need to run tests on all browsers after a change).
To satisfy the above, the special util was created which is used to generate tests based on provided fixtures (and this basically closes tickets related to testing infrastructure ckeditor/ckeditor5#2481 and ckeditor/ckeditor5#2480).

Note: As this PR was built on top of t/8, it can be merged only after #11 (t/8 PR) is merged.

@coveralls
Copy link

coveralls commented Oct 26, 2018

Pull Request Test Coverage Report for Build 108

  • 5 of 5 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 101: 0.0%
Covered Lines: 86
Relevant Lines: 86

💛 - Coveralls

import multipleStylesSingleLineModel from './multiple-styles-single-line/model.word2016.html';
import multipleStylesMultilineModel from './multiple-styles-multiline/model.word2016.html';

export const fixtures = {
Copy link
Member

Choose a reason for hiding this comment

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

I don't really understand what's in fixtures and browserFixtures and how it is used later on. I see they are combined somehow... Could you perhaps add a README.md to tests/ explaining what's the process of adding more tests using this system?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added README.md file describing briefly each part of testing mechanism and some short step-by-step guide on adding new tests 👍

@f1ames f1ames requested a review from Reinmar October 29, 2018 11:14
@Mgsy Mgsy self-requested a review November 2, 2018 09:39
@f1ames f1ames mentioned this pull request Nov 5, 2018
tests/README.md Outdated
@@ -0,0 +1,221 @@
# Normalization and integration testing in `Paste from Office`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Normalization and integration testing in `Paste from Office`
# Testing this package

@Reinmar Reinmar merged commit 51dbe7b into master Nov 5, 2018
@Reinmar Reinmar deleted the t/12 branch November 5, 2018 15:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants