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

[dashboard] Refactor What's New dialog #4607

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

corneliusludmann
Copy link
Contributor

@corneliusludmann corneliusludmann commented Jun 24, 2021

That's how it looks like for an active user that has already VSCode as editor:

Light Theme Dark Theme
image image

That's how it looks like for an active user that still has Theia as editor:

Light Theme Dark Theme
image image

After the click on “Close” or “X” the editor will be changed to VSCode. The user has to change this back in the preferences.

That's how it looks like for a user that has not been seen the previous “What's New” message because he/she has not logged in since then.

Light Theme Dark Theme
Peek 2021-06-25 17-52 Peek 2021-06-25 17-52_2

Fixes #4459

See also: #3956

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for adding this, @corneliusludmann! 🎁

Left some early feedback in the relevant discussion (internal).

components/dashboard/src/whatsnew/WhatsNew-2021-06.tsx Outdated Show resolved Hide resolved
@svenefftinge
Copy link
Member

svenefftinge commented Jun 25, 2021

The main point for this release is to inform everyone who still is using Theia that support is going to be removed. I feel this message gets a little burried within the other information of the "what's new" dialog.

Maybe we switch everyone to VScode and change the button so that people have to opt-out of that switch. They will get the new IDE and in case they did not read the "news" and still want theia they will try to switch back to it where they again get the message that we are removing support end of august.

@svenefftinge
Copy link
Member

Could you add the deprecation and end-of-life message to the settings page as well?

@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch 2 times, most recently from 3f31a3f to 3c98f8c Compare June 25, 2021 09:28
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from 3c98f8c to ec71cdb Compare June 25, 2021 10:14
@corneliusludmann
Copy link
Contributor Author

Could you add the deprecation and end-of-life message to the settings page as well?

Done here: #4614

@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from ec71cdb to 2cc38d6 Compare June 25, 2021 11:47
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Almost there! 🏁

components/dashboard/src/whatsnew/WhatsNew-2021-06.tsx Outdated Show resolved Hide resolved
components/dashboard/src/components/CodeText.tsx Outdated Show resolved Hide resolved
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from 2cc38d6 to c3546de Compare June 25, 2021 12:28
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Left behind these two minor comments! 😇

components/dashboard/src/whatsnew/WhatsNew-2021-06.tsx Outdated Show resolved Hide resolved
components/dashboard/src/whatsnew/WhatsNew-2021-06.tsx Outdated Show resolved Hide resolved
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch 2 times, most recently from 77525b2 to fdd62eb Compare June 25, 2021 15:00
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from fdd62eb to b215475 Compare June 25, 2021 15:33
@corneliusludmann
Copy link
Contributor Author

Hey @gtsiolis! Thanks a lot for all your input! ❤️

I've updated the PR as well as the PR description above with latest screenshots.

I noticed that the monospaced code texts overlap a bit with the text in the line above and below. If you have a quick fix for this handy, I can address this in this PR as well. Otherwise, I would vote for creating an issue and addressing this in a follow-up PR.

@corneliusludmann corneliusludmann marked this pull request as ready for review June 25, 2021 16:02
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for pushing this through the finish line @corneliusludmann!

Didn't manage to connect to the database but left some comments inline.

Shall we also remove the #3956 from the PR description and tackle this separately?

components/dashboard/src/components/CodeText.tsx Outdated Show resolved Hide resolved
components/dashboard/src/whatsnew/WhatsNew.tsx Outdated Show resolved Hide resolved
<button className="ml-2 mt-0 secondary" onClick={internalClose}>Dismiss All</button>
<div className="flex flex-col items-center">
<button className="ml-2" onClick={next}>Next</button>
<div className="text-xs italic mt-1 ml-2 -mb-2">({unseenEntries.length} left)</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

thought: Thinking whether we could just drop this part. Maybe something to consider in #3956. 💭

components/dashboard/src/whatsnew/WhatsNew.tsx Outdated Show resolved Hide resolved
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from b215475 to 94c838b Compare June 25, 2021 17:04
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from 94c838b to b9ccf78 Compare June 25, 2021 18:41
@corneliusludmann
Copy link
Contributor Author

I've updated the PR. When we agree on the 2 left items we are ready to merge this on Monday. 🚀

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 28, 2021

Thanks for the updates, @corneliusludmann! Left two comments for the unresolved discussions. 💬

Friendly reminder to remove #3956 from the PR description as this contains suggestions out of the scope of this PR! Let me know if this you think this is not needed. 🎗️

@corneliusludmann
Copy link
Contributor Author

Friendly reminder to remove #3956 from the PR description as this contains suggestions out of the scope of this PR! Let me know if this you think this is not needed.

Since it is not tagged with the Fixes keyword it should be fine to have it linked in the descriptions (it does not close the issue), right?

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 28, 2021

... it should be fine to have it linked in the descriptions ...

Absolutely! It's always helpful to have linked references like this. Thanks, @corneliusludmann! 🙏

@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from b9ccf78 to 31b03dc Compare June 28, 2021 07:38
@corneliusludmann corneliusludmann force-pushed the clu/theia-add-a-deprecation-4459 branch from 31b03dc to b3e2569 Compare June 28, 2021 07:40
@corneliusludmann
Copy link
Contributor Author

Thanks again, @gtsiolis!

  • Updated PR based on your comments.
  • No TODOs in the code left. 😇
  • Rebased on main.
  • Quick test of the preview env.

I think we are ready to merge. Any last-minute objections?

@gtsiolis
Copy link
Contributor

gtsiolis commented Jun 28, 2021

Any last-minute objections?

LET'S DO MERGE THIS.

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

Successfully merging this pull request may close these issues.

[theia] Add a deprecation warning
3 participants