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.

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

37 selection deletion #46

Merged
merged 4 commits into from
Aug 9, 2015
Merged

37 selection deletion #46

merged 4 commits into from
Aug 9, 2015

Conversation

bantic
Copy link
Collaborator

@bantic bantic commented Aug 6, 2015

This adds post#cutMarkers(markers) and editor#deleteSelection.

Also adds Helpers.toolbar for sharing toolbar-related assertions in tests, and
Helpers.dom.triggerDelete, which detects phantom and simulates a delete (by calling editor.handleDeletion) — this allows some of the previous skipped-in-phantom tests to run in phantom, now.

Ready for review. I will start on the items below but make a separate PR for them. @mixonic lmk if you have feedback.

To do:

// add a blank marker to any sections that are now empty
changedSections.forEach(section => {
if (section.isEmpty()) {
section.appendMarker(Marker.createBlank());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should go through the builder?

@mixonic
Copy link
Contributor

mixonic commented Aug 8, 2015

@bantic beside that comment on createMarker I think this looks mighty good. Do you want to merge?

@mixonic
Copy link
Contributor

mixonic commented Aug 8, 2015

Because if you merge before #48 I'll do the rebase for cutMarkers instead of making you do it 😉

@bantic
Copy link
Collaborator Author

bantic commented Aug 9, 2015

:) thanks! I can help with rebasing cutMarkers if you need it.

bantic added a commit that referenced this pull request Aug 9, 2015
@bantic bantic merged commit de00376 into master Aug 9, 2015
@bantic bantic deleted the 37-selection-deletion branch August 9, 2015 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants