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

FIX: Link dialog doesn't show in Safari when caret within word #2584

Merged
merged 3 commits into from Aug 30, 2017

Conversation

tg-ephox
Copy link
Contributor

fixes #2562

In Safari when the Range is collapsed getBoundingRect returns all zeros so use range of the startContainer in this instance

If a paragraph block is empty and the link toolbar is clicked the link dialog shows in the centre of the block which seems acceptable. Another issue exists to stop creation of a link if no text is selected.

@codecov
Copy link

codecov bot commented Aug 29, 2017

Codecov Report

Merging #2584 into master will increase coverage by 2.35%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2584      +/-   ##
==========================================
+ Coverage   31.59%   33.95%   +2.35%     
==========================================
  Files         175      177       +2     
  Lines        5307     5829     +522     
  Branches      916     1074     +158     
==========================================
+ Hits         1677     1979     +302     
- Misses       3080     3249     +169     
- Partials      550      601      +51
Impacted Files Coverage Δ
blocks/editable/index.js 10.52% <0%> (-0.5%) ⬇️
components/higher-order/with-api-data/request.js 97.67% <0%> (-2.33%) ⬇️
blocks/url-input/index.js 1.28% <0%> (-0.42%) ⬇️
blocks/api/parser.js 97.93% <0%> (-0.18%) ⬇️
editor/sidebar/post-taxonomies/index.js 0% <0%> (ø) ⬆️
editor/modes/visual-editor/block-list.js 0% <0%> (ø) ⬆️
editor/sidebar/post-excerpt/index.js 0% <0%> (ø) ⬆️
editor/sidebar/discussion-panel/index.js 0% <0%> (ø) ⬆️
editor/sidebar/header.js 0% <0%> (ø) ⬆️
editor/sidebar/post-status/index.js 0% <0%> (ø) ⬆️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9de9d2...726ef2f. Read the comment docs.

@tg-ephox tg-ephox self-assigned this Aug 29, 2017
@youknowriad
Copy link
Contributor

youknowriad commented Aug 29, 2017

This is causing the link modal to show up under the paragraph block (instead of under the cursor position) I tested in Chrome (and the cursor was in the middle of the paragraph block)

screen shot 2017-08-29 at 10 05 19

@tg-ephox
Copy link
Contributor Author

@youknowriad have looked at this again and I think it works quite well now. The new approach is to select a single character when the range is collapsed except if there's no text content (in the case of an empty block) where it selects the range of the container, which ends up with the link dialog centred below the empty block.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

Works great thanks for the fix

@tg-ephox tg-ephox merged commit 75d6f46 into master Aug 30, 2017
@tg-ephox tg-ephox deleted the fix/2562-link-dialog-doesnt-show branch August 30, 2017 23:37
@BoardJames BoardJames added this to Done in Ephox Team Nov 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Link dialog doesn't show in Safari when caret within word
2 participants