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

Welcome Page, About Dialog, Additional Open VSX Plugins #24

Merged
merged 5 commits into from
Jan 26, 2021

Conversation

jfaltermeier
Copy link
Contributor

@jfaltermeier jfaltermeier commented Jan 18, 2021

What it does

  • This adds a welcome page and an about dialog that explain the purpose of the blueprint application and provides useful links for the user (documentation, how to report bugs, ..)
  • Adds additional open vsx plugins in order to achieve good support for JavaScript, Typescript, and Java, and additional useful tools like markdown.
  • This increases the version to 1.9.1 so that we may test updating existing 1.9.0 application

Closes #5
Closes #9
Closes #10

ksnip_20210118-105935
ksnip_20210118-105947

Contributed on behalf of STMicroelectronics

How to test

Build application and review getting started page and about dialog.
Open a workspace and test language support

Review checklist

Reminder for reviewers

jfaltermeier and others added 4 commits January 18, 2021 10:16
Signed-off-by: Johannes Faltermeier <[email protected]>
Contributed on behalf of STMicroelectronics
* reuse code from welcome page by moving it to a util

Signed-off-by: Johannes Faltermeier <[email protected]>
Contributed on behalf of STMicroelectronics
Signed-off-by: Lucas Koehler <[email protected]>
Contributed on behalf of STMicroelectronics
* Add additional plugins
* Update plugin versions

Signed-off-by: Lucas Koehler <[email protected]>
Contributed on behalf of STMicroelectronics
@marcdumais-work
Copy link
Contributor

marcdumais-work commented Jan 18, 2021

@vince-fugnitto would you have a look at the branding aspect of this PR?

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I have some general comments about the pull-request:

  • the about dialog is not responsive and does not display properly for smaller viewports.
  • the styling of the custom getting-started can be improved (especially when verifying responsiveness).
  • we should think of updating the applicationName for the application:
    • this will be used for example in the window title.
    • we will no longer need to hardcode theia blueprint but can rather read the applicationName once.
    • we can store user level configurations under the custom name (user home - ~/.theia-blueprint) rather than the upstream default ~/.theia.

@koegel
Copy link

koegel commented Jan 20, 2021

I have some general comments about the pull-request:

Thank you for your feedback, all good and valid points!

  • the about dialog is not responsive and does not display properly for smaller viewports.
  • the styling of the custom getting-started can be improved (especially when verifying responsiveness).
  • we should think of updating the applicationName for the application:
    • this will be used for example in the window title.
    • we will no longer need to hardcode theia blueprint but can rather read the applicationName once.

I opened an issue for these points: #27
In addition there is already a ticket to update the application name property: #25

  • we can store user level configurations under the custom name (user home - ~/.theia-blueprint) rather than the upstream default ~/.theia.

I added this to the existing tickets to update all config files: #25

To me it seems none of this is a blocker and we can go ahead with the PR?

@marcdumais-work
Copy link
Contributor

To me it seems none of this is a blocker and we can go ahead with the PR?

Absolutely. I'll have a quick look at the latest version and approve ASAP.
@vince-fugnitto everything good for you?

Copy link
Contributor

@marcdumais-work marcdumais-work left a comment

Choose a reason for hiding this comment

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

This is almost ready. I do not see that the comment about adding the node-debug extensions had been resolved. I'm also ok with opening an issue or adding this item to an existing issue, as you see fit

@vince-fugnitto
Copy link
Member

@vince-fugnitto everything good for you?

Everything looks fine as an initial update to about and getting-started for me 👍

@marcdumais-work
Copy link
Contributor

@vince-fugnitto then can you please approve?

* Increase version to 1.9.1 in order to test updates
* add more debug and markdown extensions

Signed-off-by: Johannes Faltermeier <[email protected]>
Contributed on behalf of STMicroelectronics
@jfaltermeier
Copy link
Contributor Author

This is almost ready. I do not see that the comment about adding the node-debug extensions had been resolved. I'm also ok with opening an issue or adding this item to an existing issue, as you see fit

This is included by now, see line 127, 128 (https://github.com/eclipse-theia/theia-example/pull/24/files#diff-60e7eeb6bc8be48e5693ebc9be980d76bb7973ab861bf167c666b519d437f501)
The build is also green now (Linux build is a bit brittle and the windows node sometimes loses connection :-()

Copy link
Contributor

@marcdumais-work marcdumais-work left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @jfaltermeier

@marcdumais-work marcdumais-work merged commit 9022847 into eclipse-theia:master Jan 26, 2021
@koegel
Copy link

koegel commented Jan 26, 2021

Thanks for reviewing and merging!

@jfaltermeier
Copy link
Contributor Author

Thank you. @thegecko Could you please restart the master build?
Unfortunately the last build failed with a 403 when fetching from github: https://ci.eclipse.org/theia/job/theia-example/job/master/

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.

Design about dialog Add welcome page in instance Define Theia default package for installer
5 participants