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

Allow scoped file names #50

Closed
simonihmig opened this issue Mar 2, 2019 · 3 comments
Closed

Allow scoped file names #50

simonihmig opened this issue Mar 2, 2019 · 3 comments

Comments

@simonihmig
Copy link
Owner

When multiple directories (say a and b) are set up in the addons config, having a file foo.jpg in both will not work, as the image is referred to only with its filename. I.e. <ResponsiveImage image="foo.jpg"/> is ambiguous. With many images in different configurations, it might be easily possible to get into name clashes.

We should support referencing files with a path, to add a scope to them.

What about something like this:

// config/environment.js

// ...
'responsive-image': {
  sourceDir: 'assets/images/responsive',
  destinationDir: 'assets/images/generated',
  // ... default image generation settings
  quality: 80,
  supportedWidths: [2048, 1536, 1242, 1080, 750, 640, 320, 150],
  
  overrides: [ // override settings for specific files, similar to eslintrc.js
      {
        dir: 'a',
        supportedWidths: [800, 400],
        // ...
      },
      {
        dir: 'b',
        supportedWidths: [160, 80],
        // ...
      }
  ]
}

This would lead to the following file handling:

  • assets/images/responsive/foo.jpg would be written to assets/images/generated/foo***.jpg and referenced as <ResponsiveImage image="foo.jpg"/>, using the default image settings
  • assets/images/responsive/a/foo.jpg would be written to assets/images/generated/a/foo***.jpg and referenced as <ResponsiveImage image="a/foo.jpg"/>, using the specific image settings
  • assets/images/responsive/b/foo.jpg would be written to assets/images/generated/b/foo***.jpg and referenced as <ResponsiveImage image="b/foo.jpg"/>, using the specific image settings

Instead of assigning configurations by folder, we could also do that by pattern matching. I.e. instead of dir: 'a' in config item, we could do pattern: 'a/*', or any other pattern to allow different settings within a single directory, e.g. pattern: '**/*-small.jpg'.

@andreasschacht thoughts?

@abhilashlr
Copy link
Contributor

abhilashlr commented Feb 24, 2020

I tried to patch-package this addon in my local and try something similar for this problem. I did something like this:

insertMetadata(filename, imagename, width, meta) {
  ...
  if (this.metaData.hasOwnProperty(imageKey) === false) {
    this.metaData[imageKey] = { images: [] };
  }
  this.metaData[imageKey].images.push(metadata);
},
addConfigData(filename) {
  let image = path.join(this.image_options.rootURL, this.image_options.destinationDir, filename);
  if (this.configData.hasOwnProperty(image) === false) {
    this.configData[image] = this.image_options;
  }
}

So the meta generated is w.r.t the entire destinationDirPath. But here's the problem, if sourceDir = destinationDir, then as fingerprint kicks in, the meta on index.html generated using content-for is appended with the fingerprint of the actual source file. Example:

{ "a/img_1-****.jpg: { images: [...] } }

If this is acceptable, I can send a PR of what is working for me perfectly other than this scenario.

@simonihmig Thoughts?

@abhilashlr
Copy link
Contributor

On the other hand, I read from here => https://github.com/kaliber5/ember-responsive-image#options that destinationDir cannot be same as sourceDir. Is fingerprint the reason for this Note?

@simonihmig
Copy link
Owner Author

Closed by #115

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants