Skip to content

Conversation

undoingtech
Copy link
Contributor

Added dashes to remove whitespace.
Also added empty comments {% comment %}{% endcomment %} for readability.

also added empty comments for readability.
@DaricusDuncan DaricusDuncan requested review from niccokunzmann and removed request for niccokunzmann November 12, 2017 04:02
@@ -18,120 +18,126 @@
<link type="text/css" rel="stylesheet" href="../css/Ubuntu.css" media="screen"/>
<link rel="shortcut icon" href="../favicon.ico?v=2" type="image/x-icon">
<meta charset="utf-8">
<title>{{ page.title | escape }} - {{ site.data.localization.title[page.lang] }}</title>
<title>{{- page.title | escape -}} - {{- site.data.localization.title[page.lang] -}}</title>
Copy link
Member

Choose a reason for hiding this comment

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

Here, the spaces have visual meaning in the title

</script>
<style>
.clicked .toggle-hint:before {
content: "{{ site.data.localization.hint.shown[page.lang] }}";
content: "{{- site.data.localization.hint.shown[page.lang] -}}";
Copy link
Member

Choose a reason for hiding this comment

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

Here, this is not needed but also ok.

{% if p.lang == page.lang %}
{% assign step_index = step_index | plus: 1 %}
{% if p_topic == topic and same_topic_started == false %}
{% comment %}{% endcomment %}
Copy link
Member

Choose a reason for hiding this comment

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

I do not know what this line is for.

{%- assign has_next_page = false -%}
{%- assign step_index = 0 -%}
{%- assign next_page = nil -%}
{%- for p in site.pages -%}
Copy link
Member

Choose a reason for hiding this comment

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

These are the lines which make the file so much bigger.

{%- assign same_topic_started = false -%}
{%- endif -%}
<a id="page-{{- p.path -}}" class="step {%- if page == p -%}current{%- endif -%}" href="..{{- p.url -}}">{%- if page == p -%}{{- step_index -}}{%- endif -%}</a>
{% comment %}{% endcomment %}
Copy link
Member

Choose a reason for hiding this comment

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

I do not know why you put the comment there. Could you elaborate?

{% endif %}
{{- content -}}
{%- if has_next_page -%}
<a href="..{{- next_page.url -}}" class="next-page">{{- site.data.localization.footer.nextStep[page.lang] -}}</a>
Copy link
Member

Choose a reason for hiding this comment

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

Adding the - has no effect here. Did you do it for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This reply will be a reply to all the threads here. When editing the layout file, I did two things:

  1. I search-replaced the following strings:
    • "{{ " --> "{{- "
    • "{% " --> "{%- "
    • " }}" --> " -}}"
    • " %}" --> " -%}"

This took out all the whitespace between elements. However, it does not save indentation, as you requested in the solution:

Please use this to reduce the number spaces.
When you do that, please keep the indentation intact - each element should be on a new line.

Taking out all the whitespace put each <a> in the <header> on the same line. I had to add some space back so that each <a> had its own line.

  1. I added comments to make spaces after certain html elements inside the <header>.

Before these changes, the page source looked like this: view-source:https://coderdojopotsdam.github.io/regex-tutorial/en/00-01.html?

After the changes, the page source looked like this: view-source:https://undoingtech.github.io/regex-tutorial/en/00-01.html?

Note: I cannot provide a link directly to the source code, so you will have to view it yourself after you click on the link.

@niccokunzmann niccokunzmann merged commit 1745e76 into CoderDojoPotsdam:master Nov 12, 2017
@niccokunzmann
Copy link
Member

@undoingtech Thanks for your pull request! I added some comments. What do you think? I merged it so we have the improvements right away.

@niccokunzmann
Copy link
Member

I found another bug #114. Would you like to review which spaces need removal and which do not?
I would like to merge it if

  • It does not change the design of the page
  • It does not change the text on the page

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.

2 participants