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

adding the ninja theme #165

Merged
merged 3 commits into from Sep 15, 2018

Conversation

Projects
None yet
6 participants
@emitanaka
Copy link
Collaborator

emitanaka commented Sep 8, 2018

I'm adding two xaringan themes - shinobi.css and kunoichi.css (which act like -fonts.css of other themes). Both of these are powered by ninjutsu.css. For an example of the functionality see https://emitanaka.github.io/ninja-theme/kunoichi-theme-example.html

Emi Tanaka
@tcgriffith

This comment has been minimized.

Copy link
Collaborator

tcgriffith commented Sep 9, 2018

やりすぎやねん(I like it)

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Sep 9, 2018

そうだねけどたのしい。つかってね。

@tcgriffith

This comment has been minimized.

Copy link
Collaborator

tcgriffith commented Sep 9, 2018

もちろん、
でも、ブロックが多いど思う、、

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Sep 9, 2018

@pat-s

This comment has been minimized.

Copy link
Collaborator

pat-s commented Sep 9, 2018

Hi guys, would be nice if you could stick to English 🙂

@tcgriffith

This comment has been minimized.

Copy link
Collaborator

tcgriffith commented Sep 9, 2018

sorry, i was too happy that I could chat with a japanese girl kunoichi( '▿ ' )in japanese!

@yihui

yihui approved these changes Sep 14, 2018

Copy link
Owner

yihui left a comment

Sorry, I'm late (was working exclusively on the recent giant blog post, and then things piled up as usual).

image

I'm not even going to review this PR line by line because the example looks awesome to me. Only one more thing for you to do: please add yourself as ctb to DESCRIPTION and an item to NEWS.md. Thanks!

@yihui yihui added this to the v.08 milestone Sep 14, 2018

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Sep 14, 2018

Not a problem @yihui especially when it was nothing urgent. I wouldn't want you to go line by line because the code at the moment is a jungle and I know it can be better 😅
I think I have done it all - my first time proper pull request so hopefully nothing went wrong.

@yihui
Copy link
Owner

yihui left a comment

I just discovered one tiny thing that may need to be changed.

--color-opacity50: rgba(86,36,87,0.5);
--color-opacity30: rgba(86,36,87,0.3);
--logo: url("../images/R-LadiesGlobal-transparent.png");
--title-background: url("../bkgs/bg1.jpg");

This comment has been minimized.

@yihui

yihui Sep 15, 2018

Owner

Will these images with relative URLs actually work? They do not seem to be included in the PR. Please feel free to use absolute URLs like https://emitanaka.github.io/ninja-theme/images/R-LadiesGlobal-transparent.png.

This comment has been minimized.

@emitanaka

emitanaka Sep 15, 2018

Author Collaborator

That's a totally a good point! And also that absolute url.. I had no idea you could do that. That's very neat.
The shinobi theme has a similar issue except I left the url empty which I didn't think it was good coding practice but it did what I wanted it to (which is to render nothing) and still making it obvious to the user they need to add their url there. But given this will be a built-in theme I guess it would make more sense if the user defines the images in their own custom css. (Okay I've decided to just remove it except the R Ladies logo).

Comments to make it better always welcomed!

Pull request coming.

Emi Tanaka
@yihui

yihui approved these changes Sep 15, 2018

Copy link
Owner

yihui left a comment

Since this is a huge PR, I guess you may want to make changes in the future. For that reason, I have given you push access to this repo, so you can update your themes directly in the future.

@yihui yihui merged commit 21433bb into yihui:master Sep 15, 2018

2 checks passed

clahub All contributors have signed the Contributor License Agreement.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Sep 15, 2018

Thanks @yihui! That will be certainly handy!

@yihui

This comment has been minimized.

Copy link
Owner

yihui commented Oct 10, 2018

The example https://emitanaka.github.io/ninja-theme/ seems to be broken (CSS files not found), and for the example https://emitanaka.github.io/ninja-theme/kunoichi-theme-example.html, there seems to be some extra characters in the top-left corner:

image

@jvcasillas

This comment has been minimized.

Copy link
Contributor

jvcasillas commented Oct 10, 2018

Adding screenshot of kunoichi title slide for themes wiki.

kunoichi_title_slide

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Oct 10, 2018

The example https://emitanaka.github.io/ninja-theme/ seems to be broken (CSS files not found), and for the example https://emitanaka.github.io/ninja-theme/kunoichi-theme-example.html, there seems to be some extra characters in the top-left corner:

image

Not sure why css is not loading on Chrome. It loads on Firefox for me.
The character on top-left corner is related to using anicon in the YAML. This happens with icon package too. @mitchelloharawild any solution to this?

@yihui

This comment has been minimized.

Copy link
Owner

yihui commented Oct 10, 2018

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Oct 10, 2018

Yeah I got it. When I knit index.Rmd, it deletes the docs/libs/remark-css/ninjutsu.css and docs/libs/remark-css/kunoichi.css and when I knit the kunoichi example, it deletes the docs/libs/remark-css/default.css so when I fix one, the other was losing their css.

@mitchelloharawild

This comment has been minimized.

Copy link

mitchelloharawild commented Oct 11, 2018

At least for the icon package, it is because the HTML id attribute that is assigned for headers by html_document does not sanitise strings from asis output from inline chunks. So when icon returns a string containing ", it ends the id attribute and wreaks havoc.

@emitanaka

This comment has been minimized.

Copy link
Collaborator Author

emitanaka commented Oct 11, 2018

Thanks Mitch. I'm going to look deeper into this when I get a chance but if you find the solution before me please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment