-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add themes docs #69
Add themes docs #69
Conversation
_docs/plugins/themes.md
Outdated
"version": "1.0.0", | ||
"description": "A theme for The Lounge", | ||
"main": "package.json", | ||
"lounge": { |
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.
add display name?
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.
Done.
_docs/plugins/themes.md
Outdated
"name": "lounge-theme-custom", // Replace 'custom' with your theme's name | ||
"version": "1.0.0", | ||
"description": "A theme for The Lounge", | ||
"main": "package.json", |
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.
Document why we do this, and explain that an actual JS file can be used to export.
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.
I did this by adding another section below, let me know what you think.
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.
Does this, referencing a JS file that exports an object, really work?
I believe this does, but it would get stopped by that first if I understand correctly.
It's fairly common for libraries to use a key in the package.json
like Babel, but I don't remember seeing them asking to export package.json
or an object in JS, do you have an idea of how they access it?
Maybe I'm looking at this wrong, sorry if that's the case 😅
Updated this with the correct install instructions and stuff from @xPaw |
_docs/plugins/themes.md
Outdated
|
||
To install theme `lounge-theme-custom` | ||
|
||
* `npm install --prefix $LOUNGE_HOME/packages lounge-theme-custom` |
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.
Maybe until lounge install
is ready, it's worth adding export $LOUNGE_HOME=<config path of The Lounge>
and (EDIT: just setting the env var is enough) before that line?mkdir -p $LOUNGE_HOME/packages
See thelounge/thelounge#1266 (comment)
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.
I think we need to set-g
as well, though I couldn't figure out the differences with/without when using --prefix
...
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.
$ npm install --prefix $LOUNGE_HOME/packages lounge-theme-solarized
/Users/jeremie/.lounge/packages
└── [email protected]
npm WARN enoent ENOENT: no such file or directory, open '/Users/jeremie/.lounge/packages/package.json'
npm WARN packages No description
npm WARN packages No repository field.
npm WARN packages No README data
npm WARN packages No license field.
vs.
$ npm install --global --prefix $LOUNGE_HOME/packages lounge-theme-solarized
/Users/jeremie/.lounge/packages/lib
└── [email protected]
🤷♂️
EDIT: when intalling with --prefix
and --global
, themes can't be found, so whatevs. --prefix
alone is good (@xPaw, reminder to remove the global
flag from your lounge install
command).
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.
EDIT: when intalling with --prefix and --global, themes can't be found
That'll be because when installing with --global
, they get put in $LOUNGE_HOME/packages/lib/node_modules
rather than $LOUNGE_HOME/packages/node_modules
.
Definitely possible to do, but we'll need a code change for that obviously.
Happy wither way.
Maybe until lounge install is ready, it's worth adding export $LOUNGE_HOME= and mkdir -p $LOUNGE_HOME/packages (EDIT: just setting the env var is enough) before that line?
Hmmm, I was assuming that we were just referring to $LOUNGE_HOME
in the documentation as where they set their home to be, and they should replace the envar with that. I wasn't actually thinking people would actually use that. ¯\_(ツ)_/¯
_docs/plugins/themes.md
Outdated
|
||
``` | ||
{ | ||
"name": "lounge-theme-custom", // Replace 'custom' with your theme's name |
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.
Comments are not supported in package.json
, so if people copy-paste as is, that could create confusion.
Also, I don't think we should document about license, issue tracker URL, etc. Just say that themes are normal npm packages, link to npm docs (like this one), and add about the lounge
property and keywords.
I understand it's a kind thing to do though.
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.
Hmmmm alright, how about it just talk about the exports, and mention that if you add "main: package.json" to your package.json, you can have the exports in that. Might be cleaner.
@YaManicKill, I took the liberty to add a commit with changes regarding the key (with regards to thelounge/thelounge#1583). Besides that, were there any other changes you wanted to make before we can merge this? |
Oh, and I switch to using |
Yeah, I think I'm happy with this, feel free to merge if you are happy as well @astorije |
Good enough, we'll improve over time, and I'll make sure to give it a pass during doc rewrite. It's good to have it prior to that so it can be properly categorized, and so that nothing is forgotten. Thanks for this @YaManicKill! |
No description provided.