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

File namer#1784 #2114

Merged
merged 11 commits into from
Oct 27, 2020
Merged

File namer#1784 #2114

merged 11 commits into from
Oct 27, 2020

Conversation

Nielsvanpach
Copy link
Member

It's already possible to use a custom file namer on conversions.
This PR allows responsive images to use a mutual custom file namer.

@Nielsvanpach Nielsvanpach marked this pull request as ready for review October 16, 2020 13:42
Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're almost there!

{original-file-name-without-extension}___{name-of-the-conversion}_{width}_{height}.{extension}
```

Just like the conversion file names, you can use another format for naming your files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unclear. So getConversionFileName is used as the basis for the responsive image file names?

Copy link
Member Author

@Nielsvanpach Nielsvanpach Oct 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DefaultFileNamer needs 2 methods:

  • getFileName used by conversions and responsive images. For example, a timestamp prefix can be added here. This will affect both the conversion file names and the responsive image file names.
  • getConversionFileName is only used for conversions. For example file-name---thumb can be renamed to file-name_thumb. You could however also change the file name here and this will only apply on conversion files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand this fully. To me it seems strange that getFileName is also used for conversion files, as getConversionFileName already handles that. It might be best to add a couple very explicit examples to the docs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean. I'll have another look at a different solution

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated:

getFileName is removed from the FileNamer. It needs these 2 methods (which do not interfere each other):

  • getConversionFileName
  • getResponsiveImageFileName

tests/TestSupport/TestFileNamer.php Outdated Show resolved Hide resolved
tests/ResponsiveImages/ResponsiveImageTest.php Outdated Show resolved Hide resolved
Copy link
Member

@freekmurze freekmurze left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nitpick!

src/Support/FileNamer/DefaultFileNamer.php Outdated Show resolved Hide resolved
@freekmurze freekmurze merged commit f2c5a56 into v9 Oct 27, 2020
@freekmurze freekmurze deleted the file-namer#1784 branch October 27, 2020 18:42
@freekmurze
Copy link
Member

Thanks!

@Nielsvanpach Nielsvanpach linked an issue Oct 28, 2020 that may be closed by this pull request
freekmurze added a commit that referenced this pull request Oct 30, 2020
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
ivoberg pushed a commit to ivoberg/laravel-medialibrary that referenced this pull request Mar 26, 2021
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (spatie#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (spatie#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (spatie#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (spatie#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
ivoberg pushed a commit to ivoberg/laravel-medialibrary that referenced this pull request Mar 26, 2021
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (spatie#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (spatie#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (spatie#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (spatie#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
ivoberg pushed a commit to ivoberg/laravel-medialibrary that referenced this pull request Mar 26, 2021
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (spatie#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (spatie#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (spatie#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (spatie#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
ivoberg pushed a commit to ivoberg/laravel-medialibrary that referenced this pull request Mar 26, 2021
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (spatie#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (spatie#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (spatie#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (spatie#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
ivoberg pushed a commit to ivoberg/laravel-medialibrary that referenced this pull request Mar 26, 2021
* wip

* wip

* wip

* wip

* fix case

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* update javascript docs

* update javascript docs

* wip JS components documentation

* wip javascript components documentation

* wip

* wip

* wip

* Update handling-uploads-with-vue-or-react.md

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* remove translations

* wip

* wip

* wip

* Fix styling

* update props

* improve conversions (spatie#2101)

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* Fix styling

* wip

* wip

* wip

* wip

* wip

* wip

* Fix styling

* wip

Co-authored-by: freekmurze <[email protected]>

* update

* document maxSizeForPreviewInBytes

* wip

* wip

* Rework frontend docs

* Rework frontend docs

* Restore data code blocks

* wip

* wip

* Update general upload guide

* Words

* Words

* Doc updates

* wip

* Fix styling

* wip

* wip

* wip

* use v9 links

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* v9

* wip

* wip

* wip

* wip

* wip

* wip

* wip

* correct year

* wip

* docs: css installation

* wip

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* Custom CSS

* docs: update JS UI components

* wip js docs

* wip

* wip

* update validation property name

* update for Vue 3

* updates for Vue 3

* Set height attribute (spatie#2132)

* update naming

* typo

* wip

* reorder sections

* capitalize CSS

* wip

* wip

* wip

* add jsonSerialize to MediaCollection

* update slot usage for Vue 3

* fix getting old values

* Fix styling

* fix last commit

* File namer#1784 (spatie#2114)

* support a custom file namer for responsive images

* Fix styling

* apply code review suggestions

* let conversions use a new file namer class

* add documentation

* cleanup last conversion file namer parts

* apply code review suggestions

* split up conversion & responsive file namer

* remove get prefix on FileNamer methods

* Update naming-generated-files.md

* Update naming-generated-files.md

Co-authored-by: Nielsvanpach <[email protected]>
Co-authored-by: Freek Van der Herten <[email protected]>

* wip

* wip

* update docs with new endpoints

* wip

* wip

* drop laravel 6

* wip

* update vapor config

* wip

* update upload endpoint

* update paths

* allow PHP 8

* wip

* remove notice

* add enable_vapor_uploads option + restructure config file

* wip

* wip

* wip

* find last occurence of starting characters (spatie#2140)

* find last occurence of starting characters

if a filename ends with _, the responsive image will have four underscores in its name. We used to trim at the first occurence, so the collection name starts with an underscore. This way responsive image will not get rendered as expected. Now, we will trim on the last occurence and the collection name will not start with an underscore in this case.

* fix test case

Co-authored-by: Adriaan <[email protected]>
Co-authored-by: freekmurze <[email protected]>
Co-authored-by: Sebastian De Deyne <[email protected]>
Co-authored-by: Willem Van Bockstal <[email protected]>
Co-authored-by: Robin Cramer <[email protected]>
Co-authored-by: AdrianMrn <[email protected]>
Co-authored-by: Niels Vanpachtenbeke <[email protected]>
Co-authored-by: Nielsvanpach <[email protected]>
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

Successfully merging this pull request may close these issues.

Responsive file names don't follow conversion file names
2 participants