Closed janishutz closed 1 year ago
I will fix the issues rn
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?
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/impress.js@1.1.0/js/impress.js
There we go. jsdelivr is the solution
What happened?
Wait... is this due to the links that it failed?
I've just come up with a great idea to make the usage easier.
...and you have just discovered what I call "Documentation Driven Development": When someone starts writing docs for a software project, but realizes "this is stupid, it would be much better/easier/elegant if it worked like this instead". Then implement the new idea, as it not only makes the software better, but also the documentation!
Afaik, you can include a js file from raw.githubusercontent.com...
Yes, but hold your horses. The fact that you put a copy there proves that it works. But, what happens when someone does the next commit? What we need now is a deployment pipeline. Which hopefully can be implemented with github actions. In other words the online .js file should be updated automatically whenever we merge a new PR. And whenever we make a release, it should publish a versioned file representing the release. Like jsdelivr does I suppose.
Can you please file a separate ticket for that? And of course feel free to implement it
I can. There's actually no need to implement anything. Jsdelivr does this automatically.
What happened?
I'm not sure but 214ce132e3e70b64d1731d1602462e416ed3ab34 doesn't exist aywhere in Github. Did you perhaps rebase or force push something where a previous commit was overwritten?
I have not, there are no other changes. I think it may be due to the links...
Ok you're right, the commit exists in your fork. Don't know why github didn't find it when I searched.
I can. There's actually no need to implement anything. Jsdelivr does this automatically.
From Github directly? Or NPM?
Both, depends on the link you provide. If you have jsdelivr.net/gh/... then you get it from GitHub
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.
Okay. I think I cannot add anything to the releases, right?
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
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?
fixed everything you mentioned. Also I think it is important that this has a high standard
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!
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.