Altai-man / docs.raku.org

Source code of a beta version of the updated docs.raku.org website
Artistic License 2.0
11 stars 6 forks source link

Camelia Spinner Added #35

Closed CodeCanna closed 3 years ago

CodeCanna commented 3 years ago

The Camellia spinner is added. Desired behavior not achieved, spinner doesn't appear but the empty element does, and doesn't hide on code run success.`

Altai-man commented 3 years ago

Thanks a lot, I'll check it out ASAP.

Altai-man commented 3 years ago

Hmm. When you are creating a lot of elements with the same ID code-run-spinner-container (and others) in HTML, the JS code tries to find the element you ask for by ID and so it becomes pretty confused about it, I suspect.

Instead I would suggest another approach: when the spinner is necessary just create it ad-hoc using JS. Using your HTML (thanks!) I wrote this:

... (added before jQuery.ajax call)
            // Adds a spinner for the user to wait...
            $(el).closest('.raku-code').find('.code-output').each(function(i, el) {
                $(el).find('div').append(
                    $('<div/>').addClass("code-run-spinner-container")
                        .append($('<div/>').addClass("code-run-spinner")
                            .append($('<img/>').attr('id', 'camelia-spinner').attr('src', 'https://raku.org/camelia-logo.png').attr('width', '50'))
                            .append($('<p/>').text('Running Code'))
                    )
                );
            });
            jQuery.ajax('/run', {

And added HTML should be removed as well, because it is not used anymore. There is also no need to call .hide anymore, because the created element will be overwritten by the result.

I think it does the correct thing, at the very least it looks so to me. The CSS seems to do the right thing as well.

I also see some redundant merges in this branch. Can you please start the branch anew, utilize (maybe tweak?) the code above I stole from you and commit it properly? So that we had a nice git history. Also, please use the icon provided by the website instead of the one hosted on raku.org.

CodeCanna commented 3 years ago

Hmm. When you are creating a lot of elements with the same ID code-run-spinner-container (and others) in HTML, the JS code tries to find the element you ask for by ID and so it becomes pretty confused about it, I suspect.

Wow what a stupid error, you are totally right those should have at least been classes.

Thank you so much for the insight, this looks awesome!

I also see some redundant merges in this branch. Can you please start the branch anew, utilize (maybe tweak?) the code above I stole from you and commit it properly? So that we had a nice git history.

I'm sorry I'm still a little new with using Git for projects with others, I have always used git personally and solitary so I would just use git push and git pull. You want me to start another branch right? And the re-merge after applying these changes?

Altai-man commented 3 years ago

You want me to start another branch right? And the re-merge after applying these changes?

Let's see. I would do a rebase on top of the master, then interactive rebase to remove the merge commits, but this can be a bit confusing for a newcomer. The easy to understand (but a bit tiresome) way would be to yes, checkout a new branch from current master:

git checkout master # make sure we're on master
git pull upstream master # pull latest changes from this repo if you have it added as "upstream", if this errors, you'd need to add such a remote with:
git remote add upstream https://github.com/Altai-man/docs.raku.org.git # and then do a pull from upstream again
git checkout -b examples-run-spinner # create a new branch atop of the master with name `examples-run-spinner` and switch to it

Then manually apply the JS above, SCSS with correct URL (I think it can be relative url, not absolute, right? Otherwise I am asking a dumb thing), re-generate CSS, commit, then git push origin examples-run-spinner and you can create a new PR from the branch to this repo using github interface.

CodeCanna commented 3 years ago

Sorry, a hectic work week has kept me from working on this, but I'm implementing the changes now.

CodeCanna commented 3 years ago

I have committed the changes and made a new branch called Camelia-Spinner

Altai-man commented 3 years ago

Sorry, I somehow don't see a new branch with a single commit?

I took the liberty to get your commit and rebase it on master in another branch, the new PR is at https://github.com/Altai-man/docs.raku.org/pull/38 thus this one can be closed, but you are still marked as an author for the commit. Thanks a lot for your contribution!