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

Extend sidebar template to support multiple paragraphs #53

Conversation

ivanov-slk
Copy link
Contributor

Hey all :)
I am adding a minor change to the Sidebar template. Please let me know how it looks like.


Summary:

Allow variable number of paragraphs for Sidebar template's cvevent-s while retaining the original formatting exactly.

Changes:

  • Command cvevent modified to support 4 arguments (previously 5). The fourth argument is a list of comma-separated strings, e.g. {string 1, string 2, string 3}, which is parsed into the same format as the original; however allowing any number of bullets;
  • A new command introduced, cvlist, which takes a list of strings and parses it with the original formatting;
  • Some of the vspace-s are modified to exactly match the original formatting;
  • main.tex formatted - hopefully it looks ok;
  • A typo fixed in CONTRIBUTING.md.

Copy link
Owner

@jankapunkt jankapunkt left a comment

Choose a reason for hiding this comment

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

Thanks a lot @ivanov-slk for your contribution. I will check on the new feature, it sounds promising.

However, can you please revert some parts the formatting? (see my comments in the code) After implementaintg the change requests I will check again and it's very likely to be merged 👍

sidebar/main.tex Show resolved Hide resolved
sidebar/main.tex Outdated
%\hspace{-0.25\linewidth}\colorbox{bgcol}{\makebox[1.5\linewidth][c]{\hspace{46pt}\HUGE{\textcolor{white}{\uppercase{M.Sc. Jan Küster}} } \textcolor{sectcol}{\rule[-1mm]{1mm}{0.9cm}} \parbox[b]{5cm}{ \large{ \textcolor{white}{{IT Consultant}}}\\
% \large{ \textcolor{white}{{JS Fullstack Engineer}}}}
%}}
%---------------------------------------------------------------------------------------
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't indent top-level statements. They should remain,

sidebar/main.tex Outdated
\vspace{-130pt}
\hspace{0.4\linewidth}
\colorbox{bgcol}{
\parbox{0.5\linewidth}{
Copy link
Owner

Choose a reason for hiding this comment

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

However, nested-levels can be indented, that's actually improving readability

sidebar/main.tex Outdated
% CV SECTIONS AND EVENTS (MAIN CONTENT)
%
%============================================================================%
%============================================================================%
Copy link
Owner

Choose a reason for hiding this comment

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

Again, please unindent

sidebar/main.tex Outdated
%---------------------------------------------------------------------------------------
% STATUS
%----------------------------------------------------------------------------------------
\cvsection{Status}
Copy link
Owner

Choose a reason for hiding this comment

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

Again, please unindent

sidebar/main.tex Outdated
% EXPERIENCE
%----------------------------------------------------------------------------------------
\cvsection{Experience}
%---------------------------------------------------------------------------------------
Copy link
Owner

Choose a reason for hiding this comment

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

Again, please unindent and all the other top-level lines, I will not comment them all :-)

@ivanov-slk ivanov-slk force-pushed the extend-sidebar-template-to-support-multiple-paragraphs branch from c7e382c to 3a52d94 Compare April 29, 2022 07:56
@ivanov-slk
Copy link
Contributor Author

@jankapunkt , thank you for your comments; I have edited per your advice.

@jankapunkt
Copy link
Owner

@ivanov-slk thank you looks great. I will merge, please ignore the CI error, it comes from #49

@jankapunkt jankapunkt merged commit 3fb97e7 into jankapunkt:master Apr 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants