-
Notifications
You must be signed in to change notification settings - Fork 1
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
9.0 website cms theme jne #3
base: 9.0
Are you sure you want to change the base?
9.0 website cms theme jne #3
Conversation
JonathanNEMRY
commented
Oct 4, 2016
- Add website_cms_theme
- Some pep8 issues
👍 |
* provide a module to manage a theme by forms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM but need some updates. Furthermore: would you mind to add some screenshots? They could be usible for documentation too.
<xpath expr="link[last()]" position="after"> | ||
<link href="/website_cms_theme/static/src/less/layouts/form.less" | ||
rel="stylesheet" type="text/less"/> | ||
<link href="/website_cms_theme/static/src/less/assets/varriables.less" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be variables.less
and the filename should change accordingly, of course ;)
<?xml version="1.0" encoding="utf-8"?> | ||
<odoo> | ||
<!-- Assets, less, js --> | ||
<template id="website_cms_them" name="Website CMS Theme" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
website_cms_them
-> you miss an 'e'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also it's better to include "assets" both in the id and the name.