[Theme] Support for screenshots #4837
Conversation
pamil
commented
Apr 20, 2016
pamil
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Related tickets | - |
| License | MIT |
|
Ah, the one thing that was missing from my April Fool's PR #4669 👍 . Speaking of, while working on it I couldn't see how to add the theme via the admin. I had to manually create a database record, then update the current channel to reference it. |
pamil
force-pushed
the
theme/screenshots
branch
from
5477656
to
e1d9f32
Compare
| * | ||
| * @return BinaryFileResponse | ||
| */ | ||
| public function streamScreenshotAction($themeId, $screenshotNumber) |
There was a problem hiding this comment.
What about $screenshotCode? e.g. homepage, product, cart, etc. which can be equivalent to it's filename screenshots/CODE.jpg
There was a problem hiding this comment.
The main issue here is that we can have both screenshots/backend/product.jpg and screenshots/frontend/product.jpg at the same time. main.jpg also is a valid screenshot path as it's the file in theme root directory
There was a problem hiding this comment.
What about restricting the convention like:
/screenshots/main.jpg
/screenshots/backend-product.jpg
/screenshots/frontend-product.jpg
- Only one-level directory
- Screenshots under /screenshots
- Code is the same as filename (+extension?)
There was a problem hiding this comment.
I'm not sure whether we should enforce that convention, what do you think about referencing to the screenshots by a hash generated from their paths?
There was a problem hiding this comment.
You know what I was looking for in this was that we can somehow provide a developer-friendly way to ask for a screenshot. e.g. In themes tables lets show homepage.jpg. Hmmm, it needs more thinking I guess.
| @@ -89,6 +90,20 @@ private function addOptionalParentsList(ArrayNodeDefinition $rootNodeDefinition) | |||
| /** | |||
| * @param ArrayNodeDefinition $rootNodeDefinition | |||
| */ | |||
| private function addOptionalScreenshotsList(ArrayNodeDefinition $rootNodeDefinition) | |||
| { | |||
| $parentsNodeDefinition = $rootNodeDefinition->children()->arrayNode('screenshots'); | |||
There was a problem hiding this comment.
$screenshotsNodeDefinition
There was a problem hiding this comment.
I'm not too good at copypasting 😿
|
Nice! |
pamil
force-pushed
the
theme/screenshots
branch
from
698c117
to
f3a5540
Compare
pamil
force-pushed
the
theme/screenshots
branch
from
7f05e3f
to
99aa0cf
Compare
pamil
changed the title
| try { | ||
| return new BinaryFileResponse($screenshotPath); | ||
| } catch (FileNotFoundException $exception) { | ||
| throw new NotFoundHttpException(sprintf('Screenshot "%s" does not exist', $screenshotPath), $exception); |
There was a problem hiding this comment.
Missing dot at the end of exception message :)
|
Thanks Kamil! |
