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

docs(playground): Ability to use console for playground #4261

Merged

Conversation

pertrai1
Copy link
Contributor

Description:

This adds rxjs to the global namespace in the docs app so that anyone can use the console as a playground in learning and testing while looking at the docs. An example would be:

rxjs.interval(4000).pipe(rxjs.operators.take(4)).subscribe(console.log);

@niklas-wortmann
Copy link
Member

Basically I like the idea. Thanks for coming up with it, but I would be interested how much the bundle size is affected by it?! I'm not sure if many users will make use of that feature?

@pertrai1
Copy link
Contributor Author

Hey, I was going based off of an issue, ReactiveX/reactivex.github.io#374 , that I saw and thought it would be good to have that as an option if someone wants to get a quick example working. I guess someone will have to put a twitter poll out and see what the response is :-)

@pertrai1
Copy link
Contributor Author

Also, what bundle size are you referring to? This is using cdn so it should not have anything to do with bundle size of this end.....at least I don't think

@coveralls
Copy link

coveralls commented Oct 17, 2018

Pull Request Test Coverage Report for Build 8189

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.962%

Totals Coverage Status
Change from base Build 8187: 0.0%
Covered Lines: 5777
Relevant Lines: 5958

💛 - Coveralls

@niklas-wortmann
Copy link
Member

I was referring to the initial download volume when you initially visit the website. What's the difference in kb between loading the website with this playground and without?

@pertrai1
Copy link
Contributor Author

@jwo719 Hey bro, thanks for clarifying. I will take a look and get back to you

Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

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

Change the commit message to start with docs()... feat will show up as a new feature for RxJS, which isn't right, necessarily.

@pertrai1 pertrai1 changed the title feat(playground): Ability to use console for playground docs(playground): Ability to use console for playground Oct 20, 2018
@pertrai1
Copy link
Contributor Author

@benlesh changed to docs()

Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

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

I checked the js script additionally loaded is just < 30 kb therefore I don't see any problem adding it...
Thanks @pertrai1

@@ -151,6 +151,7 @@ <h2 style="color: red; text-align: center;">
<b><i>This website requires JavaScript.</i></b>
</h2>
</noscript>
<script src="https://cdnjs.cloudflare.com/ajax/libs/rxjs/6.3.3/rxjs.umd.min.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

This can be loaded asynchronously, to prevent it from blocking the main application loading. Please add the async attribute <script src="" async></script>. This will allow content loaded events to fire before that script is done loading.

@niklas-wortmann
Copy link
Member

@pertrai1 can I support you in some way to solve the requested changes? I would really like to merge this one!

This adds rxjs to the global namespace in the docs app so that anyone
can use the console as a playground in learning and testing. An example
would be:

```
rxjs.interval(4000).pipe(rxjs.operators.take(4)).subscribe(console.log);
```
@pertrai1 pertrai1 force-pushed the rxjs-global-console-playground branch from 843fe88 to 62c5e2a Compare March 3, 2019 17:28
@rsimpson2
Copy link

@jwo719 @benlesh I have made the quick change asked for.

Copy link
Member

@niklas-wortmann niklas-wortmann left a comment

Choose a reason for hiding this comment

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

LGTM

@niklas-wortmann niklas-wortmann merged commit 4378540 into ReactiveX:master Mar 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants