Skip to content

Conversation

@Hofer-Julian
Copy link
Contributor

No description provided.

@Hofer-Julian Hofer-Julian requested a review from sonnyp September 6, 2023 21:21
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Why?

@sonnyp sonnyp self-assigned this Sep 8, 2023
@Hofer-Julian
Copy link
Contributor Author

Why?

Because od the way the previewer is set up. The demos without Javascript code don't compile in Rust.
The Vala demos have the same problem.

@sonnyp
Copy link
Contributor

sonnyp commented Sep 8, 2023

I see, the problem is when opening a Library demo that is not ported yet and pressing "Run".
I don't think making it compile nothing is the right solution. With these changes it will "fail" silently and possibly confuse users.

We currently show this in the Code panel and an error in the console (at least for Rust) which is more useful.

// Sorry, this demo is not available in Rust yet.

Here are a couple of non exclusive ideas:

When the demo is not available in the currently selected Code lang

  • Force open the Code panel (to make sure the comment is visible)
  • Disable the Run button
  • Automatically Switch to JavaScript

@Hofer-Julian
Copy link
Contributor Author

I see, the problem is when opening a Library demo that is not ported yet and pressing "Run".

This is about demos that have no javascript code, not demos that are not ported yet :)

@Hofer-Julian
Copy link
Contributor Author

Should have been more specific in the PR, sorry for that.

@sonnyp
Copy link
Contributor

sonnyp commented Sep 8, 2023

Ha, gotcha.

I wonder if we shouldn't just hide the Run button in that case since it does nothing anyway and might confuse beginners. WDYT?

@sonnyp
Copy link
Contributor

sonnyp commented Sep 8, 2023

Let's chat, will be easier.

@Hofer-Julian Hofer-Julian requested a review from lw64 as a code owner September 8, 2023 19:46
@Hofer-Julian Hofer-Julian requested a review from sonnyp September 8, 2023 19:50
@Hofer-Julian
Copy link
Contributor Author

For context, @sonnyp and I agreed that the approach in this PR makes the most sense for now.
@lw64 I've also did the same for Vala, seems to work fine as far as I can tell.

Copy link
Contributor

@lw64 lw64 left a comment

Choose a reason for hiding this comment

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

If its not too much work for you, it would be nice to havea line brak after the {, but its really not important.

@lw64
Copy link
Contributor

lw64 commented Sep 8, 2023

But thanks for the work :)

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Thank you !

Regarding the line break, I'll take care of it, the Vala formatter will fix it

@sonnyp sonnyp merged commit 6514c98 into main Sep 8, 2023
@sonnyp sonnyp deleted the empty-rust-demos branch September 8, 2023 22:13
sonnyp added a commit that referenced this pull request Oct 16, 2023
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.

4 participants