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

Beta Bug: media_folder_relative also setting relative path when uploading images #2696

Closed
Jan17392 opened this issue Sep 24, 2019 · 13 comments
Closed

Comments

@Jan17392
Copy link

Jan17392 commented Sep 24, 2019

Describe the bug
I'm referring to a Gatsby issue Issue 4753, where you announced relative image paths in the beta. This works great for loading files from the static folder that already exist, but uploading a new file via the admin also saves them as relative paths to the .md file, making it impossible to find it later when loading (basically ignoring the media_folder setting).

To Reproduce

  1. Install the Beta "^2.9.8-beta.3"
  2. Set the config
    media_folder_relative: true media_folder: "static/uploads"
  3. Upload an image via the netlify admin area
  4. Check the .md file image path

Expected behavior
Uploads should be saved as image: /uploads/logo.png, but instead they're saved as ../../../uploads/logo (number of directory changes depending on where the page lives).

Applicable Versions:

  • Netlify CMS version: "^2.9.8-beta.3"
  • Git provider: Gitlab
  • OS: MacOs

CMS configuration
`
backend:
name: gitlab
repo: ********
auth_type: implicit # Required for implicit grant
app_id: *********
api_root: ********
base_url: ********
auth_endpoint: oauth/authorize
media_folder_relative: true
media_folder: "static/uploads"
public_folder: "/uploads"
collections:

  • name: navbar
    label: Navbar
    folder: src/markdown/widgets/navbar
    create: true
    fields:
    • {label: "Template Key", name: "templateKey", widget: "hidden", default: "navbar-widget"}
    • {label: "Title", name: "title", widget: "string", hint: "Please choose a unique name for the Navbar. The grey background will just be visible here, the regular navbar will be transparent"}
    • {label: Logo, name: logo, widget: object, fields: [{label: Image, name: image, widget: image}, {label: "Image Alt", name: imageAlt, widget: image}, {label: Path, name: path, widget: string}, {label: Alt Text, name: altText, widget: string}]}
    • {label: Tabs, name: tabs, widget: list, fields: [{label: Label, name: label, widget: string}, {label: Path, name: path, widget: string}, {label: Offset, name: offset, widget: string}]}
    • {label: "Call to Action", name: cta, widget: object, fields: [{label: Text, name: text, widget: string}, {label: Path, name: path, widget: string}]}
      `
@erquhart
Copy link
Contributor

This is what that functionality is supposed to do, write a relative path to the image from the entry. However, I'm just realizing that it's calculating the relative path based on the entry location in source rather than the entry location on the published site 🤦‍♂

@s0 this is based on a quick glance, let me know if I'm missing something. If not we may need to revert this puppy.

@s0
Copy link
Contributor

s0 commented Sep 24, 2019

I haven't tested this, however from briefly reading the issue and your comment, it is supposed to calculate the relative path based on the source location, and not the published site. The idea is that you only use media_folder_relative: true when using a pre-processor such as gatsby. Solving issues such as gatsbyjs/gatsby#4753

@Jan17392
Copy link
Author

@s0 Yes, I got that and that's basically what the issue that led me to trying the beta in the first place. However, it's really easy to test that something must be off, because selecting an image in the admin area even breaks the preview of it:
image

This is the resulting path that doesn't work in the admin view:
"../../../../static/uploads/kitty.png"

However, this path would work in the admin:
"/uploads/kitty.png"

To me it looks like the relative path is technically set correctly, but gatsby will only look into static anyways as this is the designated media folder and thus the path will not lead to the right location.

Let me know if this is still unclear, but for me even the plain netlify cms is not working with the intended implementation (broken preview image).

@hu0p
Copy link

hu0p commented Sep 25, 2019

If I'm following everyone correctly, it sounds like Netlify CMS needs to be aware of two types of relative paths at once when using this flag. One relative to production to enable display in the admin (sourced from the repo) and one relative to source so Gatsby can do its thing.

To elaborate: Netlify CMS needs the original, unaltered relative path from the entry location of the published site (the one it typically outputs already) to source images from the repo correctly so that it can display images in the admin.

In a Gatsby build, that would be /static/[media folder name here]/, where the root folder is the public folder. However, Gatsby needs the relative path to the image from the markdown file in source to flag the frontmatter field on the node as matching an existing file node, which subsequently adds and populates childImageSharp fields for that frontmatter field in the schema.

If that's correct, it's imperative that anyone who uses this flag with Gatsby chooses the static folder as their media folder in the CMS config to facilitate image previews. Regardless, it may not matter, because this still adds additional challenges for full panel previews, because it means the shape of the data used in the previews will never match the shape of the data used in production components.

@erquhart
Copy link
Contributor

@s0 ah, right, that's it. As a matter of fact you're all correct lol. This is a complicated issue, which is why we hadn't taken it on just yet, really needs to be thought through on all sides. I'm certain that there are ways to pull this off though. Good breakdown @hu0p, we need both a relative source path and a relative production path. Could be relative to root for the production path as mentioned by @Jan17392.

@s0
Copy link
Contributor

s0 commented Sep 26, 2019

Ah yes, I forgot that previews don't work for this, and weren't fixed in my original PR, I mention it at the end of the description here: #2394.

Rather than calculate the published path for an image, I feel it would make more sense to embed the image directly from the backend (e.g. GitHub) like is done with the gallery picker (IIRC), rather than try and resolve the published location for the image.

This is slightly more robust, and allows for e.g. staging environments (master publishes to staging and is checked there before publishing a blog live), where the image may not yet be publicly accessible even if it's in git.

@s0
Copy link
Contributor

s0 commented Sep 26, 2019

I also have a somewhat alterior motive for doing it this way, which is to get preview images for #2397 working (where an image used in a post may not even be in the default branch at all yet).

@s0
Copy link
Contributor

s0 commented Sep 26, 2019

In any case, if calculating the (site-relative) production path is preferable for everyone else, it should be fairly easy to work that out from the existing configuration options. We can work that out fairly easy:

  1. Use the Path of the entry to work out the path of the image in the repo.
  2. Use media_folder to work out the path of the image relative to the media folder (will usually just be the filename, if picked from the gallery)
  3. Use public_folder to work out the production path.

@MPratley
Copy link

MPratley commented Sep 28, 2019

Perhaps this is an over-simplification/misunderstanding, but couldn't the public_folder setting be left as is in frontmatter/markdown in certain scenarios? Particularly when using ~/ in conjunction with webpack. To illustrate the desired effect:

# Settings
media_folder: "assets/img"
public_folder: "~/assets/img"

# Result in yml/json/frontmatter
{ image: "~/assets/img/hello.jpg"}      # <- currently this becomes "/~/assets/img" 🥺

This can then be happily used in nuxt/gatsby/whatever as a psuedo-relative path, while the admin interface should still be okay finding assets since the paths aren't actually relative.

EDIT:
Of course, this can get funky inside markdown blocks, as users need to be aware that their markdown parser HAS to run before webpack or their paths will 404. So perhaps this could all go behind a setting called webpack_root: ~/, or just a generic public_folder_root: ~/

EDIT 2 (Edit Strikes Back):
I've mocked this out locally and it causes all sorts of pain and mayhem in nuxt, so maybe this wasn't such a great avenue to go down after all 😅. It sort of worked if the json only contained the filename hello.jpg and the rest of the path (~/assets/img/) is given to webpack statically.

@erezrokah
Copy link
Contributor

Sorry to jump in late into this discussion, but there might be another solution.
I'm using this Gatsby plugin to make sure the CMS paths play nice with Gatsby.
I can confirm I'm getting optimized Gatsby images just by adding the plugin, which I believe was the point of the original issue in the Gatsby repo.

@Lucianovici
Copy link

Hi, do we have a proper fix for this? :)

@erezrokah
Copy link
Contributor

As a part of this #2958 image previews will always be taken directly from the repo and not the published site.
media_folder and the path of the entry will be used to figure out where to save the image in the repository, while public_folder will be used to populate the yml/json/frontmatter to be used later by site generators.

Will work only for editorial workflow at the moment, but this #568 is next on our agenda.

@erezrokah
Copy link
Contributor

Closing as media_folder_relative was removed in favour of https://www.netlifycms.org/docs/beta-features/#folder-collections-media-and-public-folder

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

No branches or pull requests

7 participants