fix(preview): Markdown component layout breaks on copy-paste #3510
Conversation
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
| import { Directive, ElementRef, HostListener, Input, Renderer2 } from '@angular/core'; | ||
|
|
||
| @Directive({ | ||
| selector: '[textAriaResize]', |
There was a problem hiding this comment.
Rename to textAreaResize
| @HostListener('input', ['$event']) | ||
| resizeOnInput() { | ||
| const input = event.target as HTMLInputElement; | ||
| this.renderer.setStyle(this.element, 'height', '1px'); |
There was a problem hiding this comment.
Why do you set to 1px here only to set it to the scrollHeight immediately afterwards?
There was a problem hiding this comment.
When I remove a line in the text area. scrollHeight has the same height and necessary to redefine it.
There was a problem hiding this comment.
Ok I understand why you're doing it now.
I think you need to ensure as well that overflow-y is set to hidden to account for the scrollbar appearing for OS / browser settings where a scrollbar consumes space. The scrollbar will flash as well as push the text resulting possibly in an incorrect calculation. Test this on several browsers.
I don't know if this is the best solution as it's doing a re-layout of the component on each input change. I can think of other ways to do this but they are certainly more involved with separate height calculations in another element. So if this works and doesn't give any negative experiences to the user (flashing, etc...), i'm ok with it.
| #editorInput | ||
| [style.height.px]="editorInput.scrollHeight" |
There was a problem hiding this comment.
What is the purpose of this if you already have the text area resize directive?
| textAreaElement = fixture.debugElement.query(By.directive(AreaSizeDirective)); | ||
| }); | ||
|
|
||
| it('textarea should have equel content height', () => { |
| ); | ||
| }); | ||
|
|
||
| it('the component should be identified by the directived', () => { |
There was a problem hiding this comment.
directived => directive
| spyOn(textAreaElement.nativeElement, 'focus'); | ||
| expect(textAreaElement).not.toBeNull(); | ||
| fixture.detectChanges(); | ||
| setTimeout(() => { |
There was a problem hiding this comment.
Is this timeout necessary?
Async tests need to use the done() protocol.
|
|
||
| it('the component should be identified by the directived', () => { | ||
| textAreaElement.triggerEventHandler('input', null); | ||
| spyOn(textAreaElement.nativeElement, 'focus'); |
There was a problem hiding this comment.
Why add a focus spy if there's no assertions being made?
Bumbilo
force-pushed
the
markdown_component
branch
from
63e0577
to
95f8cc2
Compare
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
Bumbilo
changed the title
Bumbilo
force-pushed
the
markdown_component
branch
from
95f8cc2
to
5f55728
Compare
Bumbilo
changed the title
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
@Bumbilo please test in firefox as I observed editing an existing description loses all new line characters in the text. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
@Bumbilo Here's what happens: |
Bumbilo
force-pushed
the
markdown_component
branch
from
05c3857
to
e35a852
Compare
|
@christianvogt |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
Your screen cap shows issues with lists showing up. I tested locally in chrome and FF and experienced the same issue. I cannot create markdown lists anymore. |
- Made textaret instead of a contenteditable element
Bumbilo
force-pushed
the
markdown_component
branch
from
e35a852
to
c896f44
Compare
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'beta' failed. Please review the job results and re-run the job by commenting [e2e-test-beta] if necessary. |
|
The E2E smoke test with feature level set to 'released' failed. Please review the job results and re-run the job by commenting [e2e-test-released] if necessary. |
|
🎉 This PR is included in version 5.2.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes: openshiftio/openshift.io#3972