Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

When in centered layout, show inline diff view also in centered layout #97165

Closed
robertrossmann opened this issue May 7, 2020 · 6 comments · Fixed by #97311
Closed

When in centered layout, show inline diff view also in centered layout #97165

robertrossmann opened this issue May 7, 2020 · 6 comments · Fixed by #97311
Assignees
Labels
diff-editor Diff editor issues feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@robertrossmann
Copy link
Contributor

Version: 1.45.0-insider
Commit: d69a79b73808559a91206d73d7717ff5f798f23c
Date: 2020-05-06T08:25:01.764Z
Electron: 7.2.4
Chrome: 78.0.3904.130
Node.js: 12.8.1
V8: 7.8.279.23-electron.0
OS: Darwin x64 19.4.0

When in centered layout, opening a diff view in inline mode (as opposed to side-by-side mode) should not force the diff view into full editor width as it does for side-by-side diff views.

This is a follow-up from #94696 where we discussed this might be a bug. 🤔

Steps to Reproduce:

  1. Toggle centered layout
  2. Open a diff view in inline mode
  3. Observe diff view now occupies full editor width (as if centered layout was not enabled)

Demo:

inline-diff

Does this issue occur when all extensions are disabled?: Yes

@Tyriar Tyriar added the diff-editor Diff editor issues label May 7, 2020
@microsoft microsoft deleted a comment from vscodebot bot May 8, 2020
@isidorn
Copy link
Contributor

isidorn commented May 8, 2020

@rebornix how to check if diff is in inline view?
@robertrossmann would you like to submit a PR to fix this?
Code pointer

&& (this.editorGroupService.groups.length > 1 || (activeEditor && activeEditor instanceof SideBySideEditorInput))) {

@isidorn isidorn added feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities labels May 8, 2020
@isidorn isidorn added this to the On Deck milestone May 8, 2020
@robertrossmann
Copy link
Contributor Author

@isidorn I'm afraid this is too much for me at this moment. The centerEditorLayout() method is not fired when I switch from inline to side-by-side diff view and I could not figure out how to query the SideBySideEditorInput instance for the current display mode. Both of these events would need to be handled so that we can keep the centered layout when an inline diff is opened and we should switch to full-width layout when the diff editor's layout is switched from inline to side-by-side.

@isidorn
Copy link
Contributor

isidorn commented May 8, 2020

@robertrossmann no worries, thanks for trying. Peng might give some good tips, and I might also look into this in the future.

@petevdp
Copy link
Contributor

petevdp commented May 8, 2020

Apologies for not mentioning that I was working on this, but I implemented a fix for this in #97311

@ChrisPapp
Copy link
Contributor

ChrisPapp commented May 8, 2020

I was playing around with this too and managed to fix it, so I created a PR. My browser did not update so I didn't see your message, sorry for the duplicate PR 😅

@petevdp
Copy link
Contributor

petevdp commented May 9, 2020

I was playing around with this too and managed to fix it, so I created a PR. My browser did not update so I didn't see your message, sorry for the duplicate PR

No problem! haha both our bads I guess 🤣 Next time I'll comment beforehand

@isidorn isidorn modified the milestones: On Deck, May 2020 May 11, 2020
@isidorn isidorn added the verification-needed Verification of issue is requested label May 11, 2020
@connor4312 connor4312 added the verified Verification succeeded label Jun 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2020
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
diff-editor Diff editor issues feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
6 participants