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

feat: use template engine for styles #240

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

undefined-moe
Copy link
Contributor

As template engine have to be used to generate website, why not use it on styles to introduce more possibilities (see changes to stylus template).

Old style templates should work without any changes needed, while it's a breaking change for people depending on custom website templates as they have to rewrite to a different templating syntax.

about changes to tests: styles is empty folder before, no idea why it exists.

further tests on builtin html template might be needed.

import copy from '@tsbb/copy-template-dir';
import { deleteAsync } from 'del';
import { moveFile } from 'move-file';
import nunjucks from 'nunjucks';
Copy link
Owner

Choose a reason for hiding this comment

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

nunjucks template library seems to have been abandoned.

@jaywcjlove jaywcjlove merged commit 668989e into jaywcjlove:master Aug 30, 2024
{% for idx, linkItem in links %}
<a href="{{ linkItem.url }}">{{ linkItem.title }}</a>{% if not loop.last %} · {% endif %}
{% endfor %}
{% endif %}
Copy link
Owner

Choose a reason for hiding this comment

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

@undefined-moe The code has an issue; the loop does not generate any content.

Copy link
Owner

Choose a reason for hiding this comment

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

@undefined-moe I tried to resolve this issue but couldn't, so I've decided to roll back the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it fixed in commit undefined-moe@3fef5e1

@@ -2,17 +2,17 @@
<html lang="en">
Copy link
Owner

Choose a reason for hiding this comment

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

The template should be named index.njk; this will enable code highlighting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed, do i have to create another pull request for this?

Copy link
Owner

Choose a reason for hiding this comment

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

@undefined-moe There are multiple issues; it is recommended to debug locally, for example, the icons in the HTML are not previewing.

You're welcome to submit a new PR.

jaywcjlove added a commit that referenced this pull request Aug 30, 2024
jaywcjlove added a commit that referenced this pull request Aug 30, 2024
github-actions bot pushed a commit that referenced this pull request Aug 30, 2024
@jaywcjlove
Copy link
Owner

image

@undefined-moe There are also issues with the style output. The correct SCSS output is as follows:

$uiw-font-adobe: "\ea01";
$uiw-font-demo: "\ea02";
$uiw-font-git: "\ea03";
$uiw-font-left: "\ea04";
$uiw-font-stylelint: "\ea05";

@undefined-moe
Copy link
Contributor Author

check undefined-moe@47263bc

@undefined-moe
Copy link
Contributor Author

undefined-moe commented Aug 30, 2024

Stylus use = for assigning variables, which is different from other preprocessors, and that's why the original cssToVars doesn't meet my need and I have to design such a templating mechanism.

do have a working example but it's super tricky: https://github.com/hydro-dev/Hydro/blob/50f1ac92784240c8e6f8df9bc753735c426df273/packages/ui-default/build/main.ts#L119-L120

upd: And you are right, it's better to prefix variables with fontname.
upd2: should be classNamePrefix instead of fontname

@jaywcjlove
Copy link
Owner

@undefined-moe You are right; it should use classNamePrefix instead of fontname.

You're welcome to submit a new PR.

jaywcjlove pushed a commit that referenced this pull request Aug 30, 2024
* feat: use template engine for styles

* fix footer links

* fix style variables

* fix templates & add prefix for css variables

* update readme links
github-actions bot pushed a commit that referenced this pull request Aug 30, 2024
* feat: use template engine for styles

* fix footer links

* fix style variables

* fix templates & add prefix for css variables

* update readme links 135f84e
jaywcjlove added a commit that referenced this pull request Aug 30, 2024
github-actions bot pushed a commit that referenced this pull request Aug 30, 2024
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