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

Editing UI for Raw HTML embedding feature #8301

Merged
merged 45 commits into from
Oct 22, 2020
Merged

Editing UI for Raw HTML embedding feature #8301

merged 45 commits into from
Oct 22, 2020

Conversation

pomek
Copy link
Member

@pomek pomek commented Oct 20, 2020 •
edited
Loading

Suggested merge commit message (convention)

Internal (html-embed): Editing UI for Raw HTML embedding feature. Closes #8291.

Tests (theme-lark): Added new icons to the manual test.

Additional information

Requires: #8290.

@pomek pomek requested a review from oleq October 20, 2020 14:41
@pomek
Copy link
Member Author

pomek commented Oct 20, 2020

Embedding a manual test HTML into the editor:

image

@pomek pomek mentioned this pull request Oct 20, 2020
Merged
@Reinmar
Copy link
Member

Reinmar commented Oct 20, 2020

Let's remember to review the lang strings: https://github.com/ckeditor/ckeditor5/pull/8290/files#diff-173c07d9b0cbd6a3480b12c92fcef0e7882d810c50455d86c0192539dcc4561a.

@pomek pomek requested a review from Reinmar October 21, 2020 06:43
@pomek pomek marked this pull request as ready for review October 21, 2020 06:43
@oleq
Copy link
Member

oleq commented Oct 21, 2020

The widget needs some overflow:

@oleq
Copy link
Member

oleq commented Oct 21, 2020

The button needs some sane z-index (it's underneath the preview)

@Reinmar
Copy link
Member

Reinmar commented Oct 21, 2020

Just checking – do we have a mask over the preview to intercept all interactions? IMO, clicking the entire area should result in selecting the widget with one exception - clicking the "edit" button should work.

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020

Just checking – do we have a mask over the preview to intercept all interactions?

No, we don't.

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020

Switching the selected element after inserting an empty rawHtml element.

image

The container needs a min-height value.

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020

Two widgets nearby themselves look bad:

image

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020

@Reinmar, #8301 (comment), fixed in 14e13fa.

@oleq
Copy link
Member

oleq commented Oct 21, 2020

Some suggestions concerning CSS classes according to the code style guide:

  • raw-html -> ✅
  • raw-html--preview-enabled -> raw-html_preview_enabled
  • raw-html--display-preview -> raw-html_preview_visible
  • raw-html__switch-mode -> raw-html__mode-toggle-button
  • raw-html__source -> ✅
  • raw-html__preview -> ✅

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020

@oleq, names of the CSS classes are fixed in e89b76e.

@pomek
Copy link
Member Author

pomek commented Oct 21, 2020 •
edited
Loading

After clicking a textarea, the model selection should be set on the rawHtml element. If not, previews will not work or the command will update the wrong element.

Fixed in 4d29875.

oleq and others added 18 commits October 22, 2020 00:31
@oleq @Reinmar
Adjusted the source container padding for better readability.
3bb1f53
@oleq @Reinmar
Adjusted embed source editor when previews are unavailable to make it…
97cfb57 
… clear textarea is read-only.
@oleq @Reinmar
Added a selection handle to the embed widget.
30ed8e5
@oleq @Reinmar
Tests: Updated tests after adding a selection handle to the html embe…
06c7ed9 
…d widget.
@oleq @Reinmar
Added some vertical spacing around html embeds.
03446e7
@oleq @Reinmar
Made sure the embed toggle button has an accessible label.
c0b7436
@oleq @Reinmar
Added missing entries to contexts.json.
cf2f81e
@oleq @Reinmar
Save and cancel buttons WIP.
761c7bd
@Reinmar
Wording in API docs.
bf36bf0
@Reinmar
Renamed all classes to include the raw-html-embed part.
30acb53
@Reinmar
Aligned command names to our convention.
8ce8a8a
@Reinmar
Removed the background image as we found it too oldschool and too unr…
d8246d4 
…eadable.
@Reinmar
Improved textarea styles.
a6dcac1
@pomek @Reinmar
"data-cke-ignore-events" is removed when in preview mode.
e0b3e7f
@pomek @Reinmar
Green tests.
67fa5f3
@Reinmar
Extracted DOM renderes and cleaned up naming to make the downcast cal…
8a6d724 
…lback comprehensible.
@Reinmar
Removed editor.isReadOnly support as it's leaking listeners right now.
a18a9c2
@Reinmar
Rewrote the widget internals using a more functional approach and sin…
35c1182 
…gle source of truth.
@pomek
Copy link
Member Author

pomek commented Oct 22, 2020

Commit build:

image

PR build:

image

@pomek pomek merged commit ea1a122 into master Oct 22, 2020
@pomek pomek deleted the i/8291 branch October 22, 2020 09:07
@oleq oleq mentioned this pull request Oct 22, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleq oleq Awaiting requested review from oleq

@Reinmar Reinmar Awaiting requested review from Reinmar

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

Successfully merging this pull request may close these issues.

Improve UI for HTML embed feature
3 participants
@pomek @Reinmar @oleq