Skip to content

reorganize, add realtime validation #218

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.

Sign up for GitHub

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

Open
acao wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

reorganize, add realtime validation #218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e48a9ec
reorganize, add realtime validation
acao Jan 11, 2023
ea1259c
suggest prettier extension because there was already a config
acao Jan 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .prettierrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"singleQuote": true,
"printWidth": 100
}
6 changes: 6 additions & 0 deletions .vscode/extensions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"recommendations": [
"sissel.shopify-liquid",
"esbenp.prettier-vscode"
]
}
12 changes: 7 additions & 5 deletions _config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ languages: ["en", "de", "pl", "ru"]
exclude_from_localizations: ["assets", "admin", "vendor"]
header_themes:
- url: /about
text: about
key: about.label
- url: /news
text: articles
key: news.label
- url: /press
text: Press
key: press
- url: /events
text: events
key: events.label
- url: /works-councils
text: Works Councils
key: works_councils.label
Copy link
Member

@shushugah shushugah Jan 15, 2023

Choose a reason for hiding this comment

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

This broke the text of menu, needs to be removed or have code updated first

Copy link
Contributor Author

@acao acao Jan 16, 2023

Choose a reason for hiding this comment

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

I did not mean to push changes to this file, my bad

footer_links:
- url: /learning
text: Learning Group
Expand Down Expand Up @@ -92,7 +92,9 @@ defaults:
type: "events"
values:
layout: "event"
namespace: event
Copy link
Member

@shushugah shushugah Jan 14, 2023

Choose a reason for hiding this comment

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

Just so I don't miss anything, these namespaces aren't used yet right? or are they for generating the breadcrumb? Because even a null namespace would confirm this is not an index page, the only time you use namespace logic in this PR? For other cases like translation, this would still be useful so can keep

- scope:
type: "news"
values:
layout: "news"
namespace: news
14 changes: 14 additions & 0 deletions _includes/breadcrumbs.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{% if page.namespace != 'index' %}
<div id="breadcrumbs">
{% assign crumbs = page.url | remove: '/index.html' | split: '/' %}
Copy link
Member

@shushugah shushugah Jan 14, 2023

Choose a reason for hiding this comment

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

can remove remove: '/index.html' | since the url never includes that in our cases?

<a href="/{% if site.lang != 'en' %}{{site.lang}}{% endif %}">{% t home.label %}</a>
{% for crumb in crumbs offset: 1 %}
{% if forloop.last %}
/ {{ crumb | capitalize }}
{% else %}
/
<a href="{% assign crumb_limit = forloop.index | plus: 1 %}{% for crumb in crumbs limit: crumb_limit %}{{ crumb | append: '/' }}{% endfor %}">{{ crumb | replace: '-', ' ' | remove: '.html' | capitalize }}</a>
Copy link
Member

@shushugah shushugah Jan 14, 2023

Choose a reason for hiding this comment

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

can remove | remove: '.html' here as well

{% endif %}
{% endfor %}
</div>
{% endif %}
2 changes: 1 addition & 1 deletion _includes/head.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>{% if page.title %}{{ page.title | escape }}{% else %}{{ site.title | escape }}{% endif %}</title>
<title>{% if page.title %}{{ page.title | escape }} | {{site.title}} {% else %}{{ site.title | escape }}{% endif %}</title>
Copy link
Member

@shushugah shushugah Jan 14, 2023

Choose a reason for hiding this comment

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

Why not replace this with page.title | | site.title basically? (need to look up correct syntaxt)

Copy link
Contributor Author

@acao acao Jan 16, 2023

Choose a reason for hiding this comment

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

The idea is to improve SEO by having both the page and site title together. the pipe character appears in the title

<meta name="description" content="{{ page.excerpt | default: site.description | strip_html | normalize_whitespace | truncate: 160 | escape }}">
<style>
{% capture styles %}
Expand Down
Loading