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

removed vscode compilation & use vscode-web package #118

Closed
wants to merge 3 commits into from
Closed

removed vscode compilation & use vscode-web package #118

wants to merge 3 commits into from

Conversation

Felx-B
Copy link

@Felx-B Felx-B commented Feb 14, 2021

Hi there,
following my comment in #62 I tried to remove the vscode compilation from the project.
Here is my first attempt if you are still interested.

But some features are still missing

  • Home button / icon is no more available
  • read-only tip is displayed on all file tabs
  • Welcome page is no more customizable
  • Notification warning banner has to be done externally
  • Loader

Let me know what you think !

* removed vscode compilation
* use vscode-web package
@Felx-B Felx-B changed the title removed vscode compilation & use vscode-web package (#3) removed vscode compilation & use vscode-web package Feb 14, 2021
@Felx-B Felx-B marked this pull request as draft February 14, 2021 14:04
Comment on lines +124 to +126
<!-- <div id='load-spinner' aria-label="loading">
<div class="lds-roller"><div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div></div>
</div>
</div> -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Loader is disabled by this comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, the issue is how we delete the element after the vscode-web loaded in browser.

This patch will work:

diff --git a/resources/index.html b/resources/index.html
index 9f05f8b..c014364 100644
--- a/resources/index.html
+++ b/resources/index.html
@@ -121,12 +121,42 @@
 		<noscript title="No JavaScript Support">
 			<h1>You need to enable JavaScript to run this app.</h1>
 		</noscript>
-		<!-- <div id='load-spinner' aria-label="loading">
+		<div id='load-spinner' aria-label="loading">
 			<div class="lds-roller"><div></div><div></div><div></div><div></div><div></div><div></div><div></div><div></div></div>
-		</div> -->
+		</div>
 	</body>
 
 	<script>
+		function waitForElement(selector) {
+			return new Promise(function(resolve, reject) {
+				const element = document.querySelector(selector);
+
+				if(element) {
+				resolve(element);
+				return;
+				}
+
+				const observer = new MutationObserver(function(mutations) {
+				mutations.forEach(function(mutation) {
+					const nodes = Array.from(mutation.addedNodes);
+					for(const node of nodes) {
+					if(node.matches && node.matches(selector)) {
+						observer.disconnect();
+						resolve(node);
+						return;
+					}
+					};
+				});
+				});
+
+				observer.observe(document.documentElement, { childList: true, subtree: true });
+			});
+		}
+		waitForElement('div[role="application"]').then(function() {
+			document.querySelector('#load-spinner').remove();
+		});
+
+
 
 parseGitHubUrl = (url) => {
 			const urlObj = new window.URL(url);

@xcv58
Copy link
Collaborator

xcv58 commented Feb 15, 2021

It looks great. But the yarn watch command is failing, and when I comment this line https://github.com/Felx-B/github1s/blob/master/scripts/watch.sh#L40, the yarn watch finish immediately.

For these points:

  • Home button / icon is no more available
  • read-only tip is displayed on all file tabs

I think we could use extension to achieve the same https://code.visualstudio.com/api/extension-capabilities/extending-workbench

@conwnet
Copy link
Owner

conwnet commented Feb 15, 2021

I think we may need some ability to customize the source code of vscode, though we should do it as little as possible (convenient for merge the newer version vscode), but sometimes we really need to do this

@Felx-B
Copy link
Author

Felx-B commented Feb 15, 2021

I think I can easily manage notification banner & loader.
Welcome page is not yet customizable, but some work are planned for this iteration microsoft/vscode#116000

But home button & read-only tips I won't be able to manage it through extension points.

@conwnet What kind of stuff, do you think, we need to keep ?
Home button & read only tips seem not essential. And welcome page is almost not visible as we open Readme file by default.

All info in Welcome page can be moved to Github1s:settings panel.
Link to github repository can be moved to status bar.

@conwnet
Copy link
Owner

conwnet commented Feb 15, 2021

I think I can easily manage notification banner & loader.
Welcome page is not yet customizable, but some work are planned for this iteration microsoft/vscode#116000

But home button & read-only tips I won't be able to manage it through extension points.

@conwnet What kind of stuff, do you think, we need to keep ?
Home button & read only tips seem not essential. And welcome page is almost not visible as we open Readme file by default.

All info in Welcome page can be moved to Github1s:settings panel.
Link to github repository can be moved to status bar.

In my opinion, we should change the source code of vscode as little as we can, but I think there are somethings we should care.

If we can't change the source code of vscode, when we want do a little change for better user experience, we have to wait the vscode officially (or never achieve it), you can search NOTE@coder in the code-server, you will fount many reasons to the change the source code of vscode to build a browser version.

On the other hand, GitHub1s just want to provide a better experience for view code, we may remove some unused code in vscode for a minimal bundle in the furture,

The vscode-web is great, I think it can provide an build script for better development experience.

@Felx-B Felx-B closed this Feb 16, 2021
@Felx-B
Copy link
Author

Felx-B commented Feb 16, 2021

Ok I understand, I close this PR

@xcv58
Copy link
Collaborator

xcv58 commented Feb 16, 2021

Ok I understand, I close this PR

I did try your PR and really enjoy it because it's so fast to build and iterate.

Maybe we could do the following to finally integrate with vscode-web:

  1. Identity the places we need to change the source code of VS Code
  2. Put those features into two groups: a. doable via extension API, b. impossible via extension API
  3. Create issues for doable via extension API
  4. Identify how could we achieve impossible via extension API. I have some ideas like, add hook APIs in vscode-web, use vanilla JS to dynamically inject UI view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants