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

Fixed: #5978 support theming-webview-content #5981

Merged
merged 1 commit into from
Aug 22, 2019

Conversation

502647092
Copy link
Contributor

@502647092 502647092 commented Aug 19, 2019

Signed-off-by: MiaoWoo admin@yumc.pw

What it does

Fixed: #5978

  • support theme

How to test

  • ext install humao.rest-client
  • Send Request baidu.http
GET http://baidu.com
  • ResponseWebView have currect css style
  • change theme
  • the web view have currect css style
    temp

Review checklist

Reminder for reviewers

@502647092 502647092 requested a review from a team as a code owner August 19, 2019 09:19
@502647092
Copy link
Contributor Author

next we just get the current theme to replace.
@akosyakov
how to get current theme?

@vince-fugnitto
Copy link
Member

next we just get the current theme to replace.
@akosyakov
how to get current theme?

https://github.com/theia-ide/theia/blob/2d4c8457c09cc6ca37fc5480d14eb080a74ed85a/packages/core/src/browser/theming.ts#L110

@502647092
Copy link
Contributor Author

next we just get the current theme to replace.
@akosyakov
how to get current theme?

https://github.com/theia-ide/theia/blob/2d4c8457c09cc6ca37fc5480d14eb080a74ed85a/packages/core/src/browser/theming.ts#L110

emm but how to call it in this file
it's not in di system

@vince-fugnitto
Copy link
Member

next we just get the current theme to replace.
@akosyakov
how to get current theme?

https://github.com/theia-ide/theia/blob/2d4c8457c09cc6ca37fc5480d14eb080a74ed85a/packages/core/src/browser/theming.ts#L110

emm but how to call it in this file
it's not in di system

You can search for getCurrentTheme and even ThemeService in the codebase, and you'll find examples of other extensions and files using it.

There is a static get method returning the service.

For instance:

const theme: Theme = ThemeService.get().getCurrentTheme();

@502647092
Copy link
Contributor Author

@vince-fugnitto
after i add import { ThemeService } from '@theia/core/src/browser/theming';
plugin-ext-vscode can't build

@theia/plugin-ext-vscode: ../core/src/browser/keyboard/browser-keyboard-layout-provider.ts(18,37): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/thei
a/node_modules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: ../core/src/browser/keyboard/keyboard-layout-service.ts(18,36): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/theia/node_mo
dules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: ../core/src/browser/label-provider.ts(18,28): error TS7016: Could not find a declaration file for module 'file-icons-js'. '/home/project/TSWorkSpace/theia/node_modules/file-icons-j
s/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/file-icons-js` if it exists or add a new declaration (.d.ts) file containing `declare module 'file-icons-js';`
@theia/plugin-ext-vscode: ../core/src/browser/shell/application-shell.ts(436,56): error TS7006: Parameter 'event' implicitly has an 'any' type.
@theia/plugin-ext-vscode: ../core/src/common/keyboard/keyboard-layout-provider.ts(17,55): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/theia/node_mo
dules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: 6:40:30 PM - Found 5 errors. Watching for file changes.

@vince-fugnitto
Copy link
Member

@vince-fugnitto
after i add import { ThemeService } from '@theia/core/src/browser/theming';
plugin-ext-vscode can't build

@theia/plugin-ext-vscode: ../core/src/browser/keyboard/browser-keyboard-layout-provider.ts(18,37): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/thei
a/node_modules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: ../core/src/browser/keyboard/keyboard-layout-service.ts(18,36): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/theia/node_mo
dules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: ../core/src/browser/label-provider.ts(18,28): error TS7016: Could not find a declaration file for module 'file-icons-js'. '/home/project/TSWorkSpace/theia/node_modules/file-icons-j
s/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/file-icons-js` if it exists or add a new declaration (.d.ts) file containing `declare module 'file-icons-js';`
@theia/plugin-ext-vscode: ../core/src/browser/shell/application-shell.ts(436,56): error TS7006: Parameter 'event' implicitly has an 'any' type.
@theia/plugin-ext-vscode: ../core/src/common/keyboard/keyboard-layout-provider.ts(17,55): error TS7016: Could not find a declaration file for module 'native-keymap'. '/home/project/TSWorkSpace/theia/node_mo
dules/native-keymap/index.js' implicitly has an 'any' type.
@theia/plugin-ext-vscode:   Try `npm install @types/native-keymap` if it exists or add a new declaration (.d.ts) file containing `declare module 'native-keymap';`
@theia/plugin-ext-vscode: 6:40:30 PM - Found 5 errors. Watching for file changes.

Always import from lib never src.

@502647092 502647092 force-pushed the GH-5978 branch 2 times, most recently from 06fcf18 to a5f0eff Compare August 19, 2019 10:48
@vince-fugnitto
Copy link
Member

@502647092 can you provide the example http file used for testing in order to send a request?

@502647092
Copy link
Contributor Author

@vince-fugnitto
after i import theme service
i can't start theia
you can look the ci log

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Aug 19, 2019

The backend (node) should not import browser specific content, that is the reason for failures.
You can see more information in our code organization wiki

@502647092
Copy link
Contributor Author

The backend (node) should not import browser specific content, that is the reason for failures.
You can see more information in our code organization wiki

ok
how to get theme info from backend
i can't search theme service in node

@502647092
Copy link
Contributor Author

@akosyakov
how to get theme info from backend
i can't search theme service in node

@akosyakov akosyakov added the webviews issues related to webviews label Aug 20, 2019
@akosyakov
Copy link
Member

@502647092 It is not possible from plugin host process. We will need to do it differently somehow.

btw we should check that when a user changes the theme that webviews gets updated. With a suggested approach it seems to be written only once.

@akosyakov akosyakov added the vscode issues related to VSCode compatibility label Aug 20, 2019
@502647092
Copy link
Contributor Author

@502647092 It is not possible from plugin host process. We will need to do it differently somehow.

btw we should check that when a user changes the theme that webviews gets updated. With a suggested approach it seems to be written only once.

maybe we can add some event to handle

@akosyakov
Copy link
Member

@olexii4 Maybe you have an idea how it could be handled?

@502647092
Copy link
Contributor Author

@akosyakov
can we move replace action to Frontend
in plugin-vscode-commands-contribution.ts

@akosyakov
Copy link
Member

@502647092 you can try, i need to think about it a bit, unfortunately no time today :(

@akosyakov
Copy link
Member

@olexii4 @502647092 i had an idea, you will need to have a look how to implement it in the code:

There should be a communication channel between host window and iframe window, we could set an initial theme in the iframe window and notify about theme changes using it. In this approach you don't need to rewrite any html.

Signed-off-by: MiaoWoo <admin@yumc.pw>
@502647092
Copy link
Contributor Author

502647092 commented Aug 21, 2019

@akosyakov
temp
i find a point to change style

@502647092
Copy link
Contributor Author

@vince-fugnitto @akosyakov @theia-ide/plugin-system
who can test it

@502647092
Copy link
Contributor Author

maybe I need move code to plugin-ext-vscode
then add other css support like variables Theme Color font

@502647092
Copy link
Contributor Author

502647092 commented Aug 21, 2019

@akosyakov we can merge theme support first

@akosyakov
Copy link
Member

maybe I need move code to plugin-ext-vscode

probably it will be good, but this code is not extensible, so fine with me for now.

@olexii4 @theia-ide/plugin-system do you know whether it is desirable? how could it be achieved?

I'm testing it now, if everything is fine and noone objects merging tomorrow.

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

It works nicely for me.

@akosyakov akosyakov merged commit 865e0f9 into eclipse-theia:master Aug 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility webviews issues related to webviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vscode][webview] support theming-webview-content
3 participants