Skip to content

ViUR3#15

Open
timmytiefkuehl wants to merge 2 commits intomainfrom
viur3
Open

ViUR3#15
timmytiefkuehl wants to merge 2 commits intomainfrom
viur3

Conversation

@timmytiefkuehl
Copy link
Copy Markdown

Whole project running on ViUR3 now.

@sveneberth sveneberth self-requested a review August 5, 2022 09:28
@sveneberth sveneberth self-assigned this Aug 5, 2022
@sveneberth sveneberth requested a review from phneutral August 5, 2022 09:28
@sveneberth sveneberth added the feature New feature or request label Aug 5, 2022
Comment thread .gitignore

deploy/pyodide
deploy/vi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove these?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is standard gitignore file in viur base. The VI is served by the viur-cli.

Comment thread .gitmodules
[submodule "deploy/server"]
path = deploy/server
url = git@github.com:viur-framework/server.git
[submodule "sources/less/ignite"]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is ignite no longer a submdule?

Comment thread Pipfile
#name = "testpypi"

[packages]
viur-core = "*"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we please use a specific version?

Comment thread deploy/.gcloudignore
Comment on lines +22 to +26
/viur/docs/
/viur/CHANGELOG.md
/viur/LICENSE
/viur/README.md
/viur/.readthedocs.yml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These shouldn't be necessary, if we load the core from pip

Comment thread sources/ts/app.ts
Comment on lines -1 to -65
/**
* Remove leading indent
* Inspired by https://github.com/jamiebuilds/min-indent
*/
function minIndent(str: string): number {
const match = str.match(/^[ \t]*(?=\S)/gm);

if (!match) {
return 0;
}

return match.reduce((r, a) => Math.min(r, a.length), Infinity);
}

/**
* Remove leading indent
* Inspired by https://github.com/sindresorhus/strip-indent
*/
function unIndent(str: string): string {
const indent: number = minIndent(str);

if (indent === 0) {
return str;
}

const regex = new RegExp(`^[ \\t]{${indent}}`, 'gm');
return str.replace(regex, '');
}

function escapeHtml(html: string): string {
const div = document.createElement('div');
div.appendChild(document.createTextNode(html));
return div.innerHTML;
}

document.addEventListener('DOMContentLoaded', () => {
const menu: Element = document.querySelector('.js-menu')!;
const menuBtn: Element = document.querySelector('.js-menu-btn')!;

menuBtn.addEventListener('click', () => {
menu.classList.toggle('is-active');
menuBtn.classList.toggle('burger--to-cross');
});

document.querySelectorAll<HTMLElement>('.js-code-me').forEach((block: HTMLElement) => {
block.innerHTML = unIndent(block.innerHTML).trim();
hljs.highlightBlock(block);
// @ts-ignore
hljs.lineNumbersBlock(block);
});

document.querySelectorAll<HTMLElement>('.js-code-child').forEach((item: HTMLElement) => {
const escapedHtml = escapeHtml(unIndent(item.innerHTML).trim());

const codeNode: HTMLElement = document.createElement('code');
codeNode.classList.add('hljs', 'language-html', 'syntaxhighlighter');
codeNode.innerHTML = escapedHtml;

item.parentElement!.insertBefore(codeNode, item.nextSibling);

hljs.highlightBlock(codeNode);
// @ts-ignore
hljs.lineNumbersBlock(codeNode);
});
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove the source file?

Comment thread sources/gulpfile.js
.pipe(gulp.dest(destpaths.embedsvg));
});

gulp.task('eslint', () =>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove this task, the linting config and the husky hook?

@@ -0,0 +1,361 @@
import logging
Copy link
Copy Markdown
Member

@sveneberth sveneberth Aug 12, 2022

Choose a reason for hiding this comment

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

Not sure, why we should need allbones.py here...

Comment thread deploy/html/sitemap.html
Comment on lines +23 to +37
{{ url("s/bar", "0.9") }}
{{ url("s/box", "0.9") }}
{{ url("s/buttons", "0.9") }}
{{ url("s/components", "0.9") }}
{{ url("s/forms", "0.9") }}
{{ url("s/gettingstarted", "0.9") }}
{{ url("s/guidelines", "0.9") }}
{{ url("s/messages", "0.9") }}
{{ url("s/mixins", "0.9") }}
{{ url("s/popout", "0.9") }}
{{ url("s/popup", "0.9") }}
{{ url("s/states", "0.9") }}
{{ url("s/tables", "0.9") }}
{{ url("s/types", "0.9") }}
{{ url("s/variables", "0.9") }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
{{ url("s/bar", "0.9") }}
{{ url("s/box", "0.9") }}
{{ url("s/buttons", "0.9") }}
{{ url("s/components", "0.9") }}
{{ url("s/forms", "0.9") }}
{{ url("s/gettingstarted", "0.9") }}
{{ url("s/guidelines", "0.9") }}
{{ url("s/messages", "0.9") }}
{{ url("s/mixins", "0.9") }}
{{ url("s/popout", "0.9") }}
{{ url("s/popup", "0.9") }}
{{ url("s/states", "0.9") }}
{{ url("s/tables", "0.9") }}
{{ url("s/types", "0.9") }}
{{ url("s/variables", "0.9") }}
{% for menu in getMenu() -%}
{% for site, name in menu.items() if site != "start" -%}
{{ url("s/%s" % site, "0.9") }}
{% endfor %}
{% endfor %}

Comment thread deploy/static/js/app.js
Comment on lines +72 to +77
// Burger Menu Mobile
$('.js_burger').click(function() {
$('.mainmenu').toggleClass("is-open");
$('.burger').toggleClass("");
$('html').toggleClass("no-scroll");
});
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This project doesn't uses jQuery and there's is no .js_burger element.

Comment thread deploy/render/__init__.py
Comment on lines +1 to +18
from viur.core.render import admin, html, json, vi, xml

@html.utils.jinjaGlobalFilter
def isList(render, val):
return isinstance(val, list)

@html.utils.jinjaGlobalFilter
def isDict(render, val):
return isinstance(val, dict)

# -*- coding: utf-8 -*-
import render.html
from server.render import admin, json, vi
import os
from collections import OrderedDict

from viur.core import conf, request, utils
from viur.core.render.html import default as defaultRender
from viur.core.render.html.utils import jinjaGlobalFunction

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can please clean up the order of imports and methods...

Comment thread deploy/ignite-docs.py
Comment on lines -39 to -58
def requestPreprocessor(path):
def isWhitelisted():
reqPath = request.current.get().request.path
for testUrl in conf["viur.noSSLCheckUrls"]:
if testUrl.endswith("*"):
if reqPath.startswith(testUrl[:-1]):
return True
else:
if testUrl == reqPath:
return True
return False

url = request.current.get().request.url
if "://ignite.viur.is" in url and not isWhitelisted():
raise errors.Redirect(url.replace("://ignite.viur.is", "://ignite.viur.dev"), status=301)

return path


conf["viur.requestPreprocessor"] = requestPreprocessor
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you please keep the requestPreprocessor?

@sveneberth sveneberth linked an issue Aug 12, 2022 that may be closed by this pull request
3 tasks
@sveneberth sveneberth removed their assignment Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Port to ViUR-Core

2 participants