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

Bad layout for images in diff view #96968

Closed
mjbvz opened this issue May 5, 2020 · 2 comments
Closed

Bad layout for images in diff view #96968

mjbvz opened this issue May 5, 2020 · 2 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release verified Verification succeeded webview Webview issues
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented May 5, 2020

Issue Type: Bug

Repro

  1. In a git repo
  2. Edit an image
  3. Try viewing the diff

Bug
The webview for the image preview can be poorly laid out (or hidden entirely)

VS Code version: Code - Insiders 1.45.0-insider (fd5d9a3, 2020-05-04T09:21:17.770Z)
OS version: Darwin x64 19.4.0

System Info
Item Value
CPUs Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz (16 x 2400)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
oop_rasterization: disabled_off
protected_video_decode: unavailable_off
rasterization: enabled
skia_renderer: disabled_off_ok
video_decode: enabled
viz_display_compositor: enabled_on
viz_hit_test_surface_layer: disabled_off_ok
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 32.00GB (0.34GB free)
Process Argv -psn_0_41998347
Screen Reader no
VM 11%
@mjbvz mjbvz added bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release webview Webview issues labels May 5, 2020
@mjbvz mjbvz self-assigned this May 5, 2020
@mjbvz mjbvz added this to the April 2020 milestone May 5, 2020
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 5, 2020

This was caused by ea07e9b

@sbatten (not sure if you are the correct owner) For May, I suggest we investigate why the .layout call has bogus dimensions. That would let me remove the workaround I added in ea07e9b Alternatively, if there's a way to find out if an editor is not visible—such as being under the panel—we could use that to determine if we should show the webview or not

For April, I'll add a ugly workaround that should be safer

mjbvz added a commit that referenced this issue May 5, 2020
Fix for #96968

This is a scoped fix for #96968.

The cause of the issue is the following:

1. Webview must be rendered outside of the main editor DOM. We do this by absolutely positioning them over some element in the DOM.
1. In split views, we try to lay the webview out over an element that has 0 height.
1. Due to my workaround in ea07e9b, this causes the webview to either not show at all (because it also will have zero height) or partially show

This fix forces the webivew's parent in the split view to have 100%. That actually seems like a reasonable default but I've scoped my fix to just webviews
mjbvz added a commit to mjbvz/vscode that referenced this issue May 5, 2020
Fix for microsoft#96968

This is a scoped fix for microsoft#96968.

The cause of the issue is the following:

1. Webview must be rendered outside of the main editor DOM. We do this by absolutely positioning them over some element in the DOM.
1. In split views, we try to lay the webview out over an element that has 0 height.
1. Due to my workaround in ea07e9b, this causes the webview to either not show at all (because it also will have zero height) or partially show

This fix forces the webivew's parent in the split view to have 100%. That actually seems like a reasonable default but I've scoped my fix to just webviews
mjbvz added a commit to mjbvz/vscode that referenced this issue May 5, 2020
Fix for microsoft#96968

This is a scoped fix for microsoft#96968.

The cause of the issue is the following:

1. Webview must be rendered outside of the main editor DOM. We do this by absolutely positioning them over some element in the DOM.
1. In split views, we try to lay the webview out over an element that has 0 height.
1. Due to my workaround in ea07e9b, this causes the webview to either not show at all (because it also will have zero height) or partially show

This fix forces the webivew's parent in the split view to have 100%. That actually seems like a reasonable default but I've scoped my fix to just webviews
mjbvz added a commit that referenced this issue May 5, 2020
Fix for #96968

This is a scoped fix for #96968.

The cause of the issue is the following:

1. Webview must be rendered outside of the main editor DOM. We do this by absolutely positioning them over some element in the DOM.
1. In split views, we try to lay the webview out over an element that has 0 height.
1. Due to my workaround in ea07e9b, this causes the webview to either not show at all (because it also will have zero height) or partially show

This fix forces the webivew's parent in the split view to have 100%. That actually seems like a reasonable default but I've scoped my fix to just webviews
mjbvz added a commit that referenced this issue May 5, 2020
Removes the workaround for maximizing the panel. Instead track if the editor is visible and only lay the view out if the editor is visible
@mjbvz
Copy link
Collaborator Author

mjbvz commented May 5, 2020

Merged fix into 1.45

Checked better fix into master with f761228

@mjbvz mjbvz closed this as completed May 5, 2020
@connor4312 connor4312 added the verified Verification succeeded label May 6, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug candidate Issue identified as probable candidate for fixing in the next release verified Verification succeeded webview Webview issues
Projects
None yet
Development

No branches or pull requests

2 participants