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

Add competency icons #203

Merged
merged 23 commits into from
Mar 2, 2024
Merged

Add competency icons #203

merged 23 commits into from
Mar 2, 2024

Conversation

BeastyBlacksmith
Copy link
Collaborator

@BeastyBlacksmith BeastyBlacksmith commented Nov 17, 2023

This adds the icons as presented in #138 (comment) throughout the document and also for the forgotten SP and USERS competencies.

The results can be seen in the compiled artifacts

@BeastyBlacksmith
Copy link
Collaborator Author

Preview from toc:

image

@BeastyBlacksmith BeastyBlacksmith added enhancement New feature or request competencies labels Nov 17, 2023
@CaptainSifff
Copy link
Collaborator

cool! maybe, after the abbreviations?

Copy link
Collaborator

@jngrad jngrad left a comment

Choose a reason for hiding this comment

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

From the technical side, pandoc is clever enough to wrap the LaTeX code for \faicon into a \texrorpdfstring inside of titles, so that no reserved UTF-8 characters appear in the PDF bookmarks (metadata is important for accessibility).

But currently, there is an extra whitespace inside the round brackets in the PDF bookmarks. In addition, the whitespace between the icon and abbreviation is a breaking space, so its easy for an icon to appear at the end of a line and the abbreviation at the beginning of the next line.

One can solve both issues at once with this modification:

diff --git a/competencies.md b/competencies.md
index 71b289f..59cbbfc 100644
--- a/competencies.md
+++ b/competencies.md
@@ -34,3 +34,3 @@ header-includes:
     - '\newfontfamily\FA[Path=../fonts/, Extension=.otf]{Font Awesome 6 Free-Solid-900}'
-    - \newcommand*{\faicon}[1]{{\FA \symbol{#1}}}
+    - \newcommand*{\faicon}[1]{{\FA \symbol{#1}}~}
 xnos-cleveref: True

but one also has to erase all whitespaces, i.e. write \faicon{"F0E8}DOCBB instead of \faicon{"F0E8} DOCBB everywhere...

@BeastyBlacksmith
Copy link
Collaborator Author

would it also work if I write \faicon{"F0E8}~DOCBB instead of \faicon{"F0E8} DOCBB everywhere?

I'm not a big fan of macros introducing their own whitespace.

@jngrad
Copy link
Collaborator

jngrad commented Nov 17, 2023

would it also work if I write \faicon{"F0E8}~DOCBB instead of \faicon{"F0E8} DOCBB everywhere?

I'm not a big fan of macros introducing their own whitespace.

The ~ symbol would print a tilde instead of a non-breaking space, since it's not recognized as a LaTeX command. I tried out \newcommand*{\faicon}[2]{\texorpdfstring{{\FA \symbol{#1}}~#2}{#2}} to provide a safe alternative for titles, but then the abbreviation disappears from the round brackets in the PDF bookmarks, because pandoc wraps it into its own \texorpdfstring.

@BeastyBlacksmith
Copy link
Collaborator Author

Does f758784 work for you?

@jngrad
Copy link
Collaborator

jngrad commented Nov 17, 2023

Does f758784 work for you?

It does, thanks!

@jngrad
Copy link
Collaborator

jngrad commented Nov 17, 2023

Good news: these icons are also compatible with a glossary via tomncooper/pandoc-gls, should we decide to include one.

competencies.md Outdated
@@ -340,7 +356,7 @@ to the FAIR principles. [@ChueHong2014] defines different levels of research
software reusability and the extent to which the software engineering skills
need to be applied to reach them.

### Creating documented code building blocks (DOCBB)
### Creating documented code building blocks (\DOCBB)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note: LaTeX commands are replaced by an empty string in the PDF bookmarks, so this heading reads "Creating documented code building blocks ()". As a workaround, one can create a dummy LaTeX macro \newcommand{\donothing}[1]{#1} in header-includes and write here \donothing{(\DOCBB)} to prevent the parentheses from appearing in the PDF bookmarks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It would also be possible using a construction from #210 that creates section headings from glossary expansions that don't generate links (to avoid issues with hyperref), like so:

% preamble
\newglossaryentry{DOCBB}{name={\faicon{"E4C7}\nobreakspace{}DOCBB},sort={DOCBB},type={skills},
                         description={Creating documented code building blocks}}
\newcommand{\skillsection}[1]{\hypertarget{skills-#1}{%
\subsubsection{\glsentrydesc{#1} (\glsentrytext{#1})}\label{skills-#1}}}

% body
\skillsection{DOCBB} % instead of `### Creating documented code building blocks (DOCBB)`

@BeastyBlacksmith
Copy link
Collaborator Author

Todays call mentioned to use the RC icon for SWLC and a doctorate hat for RC.

Might use a light bulb, question mark or brain for NEW (if the that stays).

@BeastyBlacksmith
Copy link
Collaborator Author

New set of icons:

image

@BeastyBlacksmith
Copy link
Collaborator Author

How badly do we need to keep using pdflatex or was it just for arXiv?

@BeastyBlacksmith
Copy link
Collaborator Author

So I added the svgs directly, but not sure, what he currently doesn't like about that.

He complains about

! Missing \endcsname inserted.
<to be read again> 
                   \let 
l.577 ...0pt]{fonts/sitemap}\nobreakspace{}DOCBB}}

However, I can compile the MWE

\documentclass{article}

\usepackage{svg}

\newcommand*{\SWLC}{\includesvg[width=10pt]{{fonts/arrows-spin}}\nobreakspace{}SWLC}
\begin{document}
\section*{\SWLC}
\SWLC
\end{document}

just fine using pdflatex --shell-escape granted that inkscape is available.

@jngrad you got an idea?

@BeastyBlacksmith
Copy link
Collaborator Author

I think this is ready for merge

Copy link
Collaborator

@jngrad jngrad left a comment

Choose a reason for hiding this comment

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

There is an issue with the pdflatex command that triggers fatal errors in the CI pipeline due to permissions. That's an easy fix. There are accessibility issues in the PDF bookmarks, I'll work on it now.

competencies.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@CaptainSifff CaptainSifff left a comment

Choose a reason for hiding this comment

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

I like them! But I wonder if their sizing is OK. It looks OK in the tables.
But I feel that the icons are a bit large in the standard Text. They are markedly taller that a capital letter. Is this intentional?

@jngrad
Copy link
Collaborator

jngrad commented Feb 27, 2024

I like them! But I wonder if their sizing is OK. It looks OK in the tables. But I feel that the icons are a bit large in the standard Text. They are markedly taller that a capital letter. Is this intentional?

They used to be bigger in the text, and would locally increase the line height. But the last commit introduced a relative height metric so they would be the same height as capital letters. Below is an illustration of the body height with equispaced rulers (red for the baseline and grey for cap height) that shows linespacing isn't affected by icons:

snapshot of the manuscript with red and grey rulers to measure the height of letters and icons

@CaptainSifff
Copy link
Collaborator

CaptainSifff commented Mar 1, 2024

OK, outdated artifacts link. THis one is from the last:
https://github.com/CaptainSifff/paper_teaching-learning-RSE/actions/runs/8021662140/artifacts/1270171265

and it looks as intended! Cool! Thanks to both of you!

@CaptainSifff CaptainSifff self-requested a review March 1, 2024 20:54
@BeastyBlacksmith BeastyBlacksmith merged commit 6ec351e into main Mar 2, 2024
5 checks passed
@CaptainSifff
Copy link
Collaborator

@jngrad arxiv doesn't like the required shell-escape of the svg package. Any idea, how I can get pdflatex to use the the already generated .pdf and .pdf_tex files?

@jngrad
Copy link
Collaborator

jngrad commented Apr 12, 2024

@jngrad arxiv doesn't like the required shell-escape of the svg package. Any idea, how I can get pdflatex to use the the already generated .pdf and .pdf_tex files?

I'm currently in a meeting and can have a look in about 1 hour. Could you try sending ArXiv the converted .pdf files instead of the original .svg files, and drop the shell escape from the build command?

@CaptainSifff
Copy link
Collaborator

No worries. It seems that the svg package requires this. Replacing includesvg with imcludeinkscape, i couldn't get that to work. Also the svg package creates some wrapper latex that seems to be used.

@jngrad
Copy link
Collaborator

jngrad commented Apr 12, 2024

The -shell-escape option is only needed if the build/svg-inkscape/*.pdf* files don't already exist. When uploading the source files to arXiv, make sure to include these files, and pdflatex should skip the call to inkscape through a subshell. Unless arXiv is changing source file timestamps? I have colleagues who like to do that kind of LaTeX "pre-processing", e.g. by submitting the .bbl file instead of the .bib file, so that the bibliography appears exactly like on their local workstation, regardless of which version of natbib is being used by publishing platforms.

@CaptainSifff
Copy link
Collaborator

Thank you!
I managed to get it work. On arxiv (or the way I created the tarball) required to modify the path in
- \newcommand{\fonticon}[2]{\includesvg[height=1.5ex]{{../fonts/#1}}\nobreakspace{}#2}
to
- \newcommand{\fonticon}[2]{\includesvg[height=1.5ex]{{./fonts/#1}}\nobreakspace{}#2}

@BeastyBlacksmith BeastyBlacksmith deleted the bbs/icons branch April 15, 2024 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
competencies enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants