homebrewery icon indicating copy to clipboard operation
homebrewery copied to clipboard

Add local statics for images and typefaces

Open dbolack-ab opened this issue 1 year ago • 10 comments

This solves issue #1958.

This adds the endpoints /staticImages and /staticFonts when Homebrewery is run as a local NODE_ENV.

staticImages is set to the path set in the HB config/default.json as hb_images OR $HBROOT/staticImages if not set. staticFonts is set to the path set in the HB config/default.json as hb_fontss OR $HBROOT/staticFonts if not set.

Paths should be fully qualified in the config file.

dbolack-ab avatar Nov 08 '23 02:11 dbolack-ab

I only worked with the images part for now, but I couldn't get it to work? This is likely my fault, so I probably just need a little more guidance.

I have pulled your PR here, confirmed that is what I am working with, and did npm install just in case. Then, I added a folder to the root directory .../homebrewery/staticImages and dropped a png in there. For now, I left the /homebrewery/config/defaults.json alone, relying on the built-in default you've added.

In my brew, I did this:

![cat warrior](Class-Table-Frame.png)

![cat warrior](staticImages/Class-Table-Frame.png)

And a couple of other variants. No dice. I then tried adding the hb_images (and HB_IMAGES) key to the default.json to see if I could get it to work that way, again no dice.

I also tried simplifying that line down to:

	app.use(express.static('./staticImages'));

Any tips?

Gazook89 avatar Feb 08 '24 04:02 Gazook89

![cat warrior](Class-Table-Frame.png)

![cat warrior](staticImages/Class-Table-Frame.png)

Any tips?

Those two url references would be relative to the current page location, and thus expanded to something like https://homebrewery.naturalcrit.com/edit/:id/Class-Table-Frame.png or https://homebrewery.naturalcrit.com/edit/:id/staticImages/Class-Table-Frame.png or https://homebrewery.naturalcrit.com/share/:id/Class-Table-Frame.png or https://homebrewery.naturalcrit.com/new/Class-Table-Frame.png.

Try ![cat warrior](/staticImages/Class-Table-Frame.png) to cause the img src to be an absolute url.

Here's an example of another image that should already work: https://homebrewery.naturalcrit.com/assets/redTriangle.png ![red triangle](/assets/redTriangle.png)

ericscheid avatar Feb 08 '24 06:02 ericscheid

Yeah I thought I tried that as well with no luck but I will try again later today. I had also tried changing the code to match what @G-Ambatte had suggested in the linked issue, and then following that format, to no avail.

Gazook89 avatar Feb 08 '24 13:02 Gazook89

What happens if you right-click the broken image and say "open image in new tab" .. what does the window.location look like?

ericscheid avatar Feb 08 '24 15:02 ericscheid

I'm getting 406 responses when attempting to get a local image file, which appears to be from the content-negotiation middleware, That said, disabling said middleware does NOT result in the image being correctly served, so I suspect that there's still something that I've missed.

G-Ambatte avatar Feb 08 '24 19:02 G-Ambatte

~~As noted in gitter, when i copy the images to the build folder, it works.~~ see below in edit

In the buildHomebrew.js script:

	// Move assets
	await fs.copy('./themes/fonts', './build/fonts');
	await fs.copy('./themes/assets', './build/assets');
	await fs.copy('./client/icons', './build/icons');
	await fs.copy('./staticImages', './build/staticImages');

(note, this in the "themes" portion, which doesn't quite sit right with me...this is sort of out of my depth, though).

and in app.js:

	// Add Static Local Paths
	app.use(express.static('build/staticImages'));

but it does require including the directory:

![alt](staticImages/image.png)

duh, of course it works...it doesn't even need the app.js bit...because it's just referencing a directory in the build, which isn't anything special. So I guess....if someone is working locally, is there anything particularly wrong with just telling them to create a path ./build/images/ and using that to store their images? Do we need have this in app.js at all?

Sorry, I think i've lost the thread.

Gazook89 avatar Feb 08 '24 20:02 Gazook89

if someone is working locally, is there anything particularly wrong with just telling them to create a path ./build/images/ and using that to store their images?

https://github.com/naturalcrit/homebrewery/blob/c27f5d9efa66f20fed3f04dfad01de3630039d49/scripts/buildHomebrew.js#L43

This line deletes the contents of /build/ every time the build action runs, so we should not recommend that the users store anything inside that directory.

I suspect that if we follow the recommendation below and use an absolute path to the staticImage directory, we'll resolve the issue without needing to copy the folder to the build directory at all. image

G-Ambatte avatar Feb 08 '24 23:02 G-Ambatte

~As noted in gitter, when i copy the images to the build folder, it works.~ see below in edit

In the buildHomebrew.js script:

	// Move assets
	await fs.copy('./themes/fonts', './build/fonts');
	await fs.copy('./themes/assets', './build/assets');
	await fs.copy('./client/icons', './build/icons');
	await fs.copy('./staticImages', './build/staticImages');

(note, this in the "themes" portion, which doesn't quite sit right with me...this is sort of out of my depth, though).

and in app.js:

	// Add Static Local Paths
	app.use(express.static('build/staticImages'));

but it does require including the directory:

![alt](staticImages/image.png)

duh, of course it works...it doesn't even need the app.js bit...because it's just referencing a directory in the build, which isn't anything special. So I guess....if someone is working locally, is there anything particularly wrong with just telling them to create a path ./build/images/ and using that to store their images? Do we need have this in app.js at all?

Sorry, I think i've lost the thread.

Sorry, seem to have missed these questions.

My read on the original request was this was desired for situations where the user probably does not want or cannot copy the static content into place or needs to point at a path outside of /build/ such as a mounted docker volume.

I've updated the initial commit comment to be a little more clear on usage.

dbolack-ab avatar Mar 04 '24 20:03 dbolack-ab

After reading all the comments, i am still not clear on if this is working and how to test this, just create a folder called staticImages and use paths relative to that?

5e-Cleric avatar Aug 25 '24 11:08 5e-Cleric