Conversation
|
I will fix the issues rn |
Fixed an error and added more infos
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Hey thanks this is amazing! I will have to read it with thought tomorrow. Thanks for writing this!
This reverts commit 40a3766.
|
I have not really used PRs before, so I'm not sure how to have multiple PRs from the same repo (so my fork where I pushed the changes). It appears that whenever I push to that repo, the PR updates... I will look up how to do it today and will update the PR asap |
|
I just got it... it works by adding additional branches right? |
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Thank you!
I made a few comments which you may or may not want to incorporate. Since this is not a code review, I'm fine leaving the final editorial judgement to you, but wanted to add some additional viewpoints where I had any.
janishutz
changed the title
|
I've just come up with a great idea to make the usage easier. Afaik, you can include a js file from raw.githubusercontent.com... If this is so, one can just copy that link with a script tag into the head of the html file and therefore you don't have to download it. |
|
Doesn't appear to work. I might be able to pull something off that is slightly different. I have only tested it on my iPad so this might be the problem as well. |
|
https://cdn.jsdelivr.net/npm/[email protected]/js/impress.js There we go. jsdelivr is the solution |
|
What happened? |
|
Wait... is this due to the links that it failed? |
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Ok so looks good otherwise, but now we can fine tune the jsdelivr version of impress.js. This conversation can also continue in the separate ticket.
|
I just changed the wording regarding jsDelivr to better explain the usage. |
|
I feel like we should also start to provide .min.js files for the cdn. I could update the package.json to include minify (or browserify for that matter) |
|
Note that build.js already creates a minified file. It's just not committed to the git repo. But adding it to release pages and CDN is a great idea. |
|
Okay. I think I cannot add anything to the releases, right? Also, what do you think about the latest changes? |
|
LOL that was weird. I commented on my phone which at school has really bad connection, GitHub spat out a network error, but my comment got actually published. |
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Some more comments on the text
Ah right. Can you file a new ticket and me or someone else could look into it in the future. |
|
I have updated my repo to include the changes you mentioned |
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Thanks. You missed a couple and I added 2 more comments.
|
I saw it, thank you |
|
I just checked again that I didn't miss any of the previously mentioned things, I didn't |
|
I have also bumped the version number to 2.1.0 in the impress.js file. It was still on V1.1.0. I might be wrong with that version number, but it'd make sense, as 2.0.0 is already out. |
|
Also, I just noticed, that the copyright info is really outdated in the README.md. Shall I update it, or will somebody else do it? |
henrikingo
left a comment
henrikingo
left a comment
There was a problem hiding this comment.
Thanks. This is really good now.
Sorry, I'm going back and forth I guess, but I have to ask you to change the part about CSS some more. Other than that I'm quite satisfied.
|
fixed everything you mentioned. Also I think it is important that this has a high standard |
Yes, I appreciate your dedication and patience. It's not so much that I hesitate to strive for perfection... I just feel bad if you've waited for review 4-6 weeks and then I ask to change one more thing. But now this is done! Thanks so much for doing this! |
2 participants
For me, getting started with impress.js was not that easy, honestly, in retrospective, I don't know why. So I created a little "Getting Started" guide to help new users starting out. I also noticed a problem, where the notes didn't hide as they should have. So I added some code that takes care of that (it just sets the CSS display property to "none" on all objects with class "notes". Tested it, it worked.)
I will also be creating an introduction video in a short while and link it in the Getting Started Guide.