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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃帹 Change asset path to /ghost/assets #309

Merged
merged 1 commit into from Oct 7, 2016

Conversation

Projects
None yet
3 participants
@ErisDS
Member

ErisDS commented Oct 6, 2016

See TryGhost/Ghost#7503 for a hopefully clear explanation of why I'm making this change

I have changed to having both an admin and an asset path because, although I can't find any usages of admin, I didn't want to hijack the term "admin" to mean "asset".

@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Oct 6, 2016

Member

This passes the tests, however it was screwing up code injection because codemirror is included using the lazyLoader.

I've fixed this & pushed up my fix, however the concatenated codemirror.js file has a source map url at the bottom:

//# sourceMappingURL=codemirror.map

And this is wrong :(

I have no idea how to make the url be correct. I tried looking at various packages that seem to be used, but none of them appear to have any documentation on how to configure them!

Any ideas?

Member

ErisDS commented Oct 6, 2016

This passes the tests, however it was screwing up code injection because codemirror is included using the lazyLoader.

I've fixed this & pushed up my fix, however the concatenated codemirror.js file has a source map url at the bottom:

//# sourceMappingURL=codemirror.map

And this is wrong :(

I have no idea how to make the url be correct. I tried looking at various packages that seem to be used, but none of them appear to have any documentation on how to configure them!

Any ideas?

@kevinansfield

This comment has been minimized.

Show comment
Hide comment
@kevinansfield

kevinansfield Oct 7, 2016

Collaborator

I believe you can add this line to the codemirror concat config to specify the relative path for the sourcemap:

sourceMapConfig: {sourceRoot: 'proper/path/'}

broccoli-concat uses https://github.com/mozilla/source-map under the hood so any options passed in via sourceMapConfig are passed through to that.

Collaborator

kevinansfield commented Oct 7, 2016

I believe you can add this line to the codemirror concat config to specify the relative path for the sourcemap:

sourceMapConfig: {sourceRoot: 'proper/path/'}

broccoli-concat uses https://github.com/mozilla/source-map under the hood so any options passed in via sourceMapConfig are passed through to that.

@kevinansfield

This comment has been minimized.

Show comment
Hide comment
@kevinansfield

kevinansfield Oct 7, 2016

Collaborator

Alternatively, that source map isn't particularly useful so it could be set to sourceMapConfig: {enabled: false}

Collaborator

kevinansfield commented Oct 7, 2016

Alternatively, that source map isn't particularly useful so it could be set to sourceMapConfig: {enabled: false}

馃帹 Change asset path to /ghost/assets
refs #7503

- Having assets served from the same directory as the admin makes this tricky to refactor server side
- It's also much harder to optimise for 404s
@ErisDS

This comment has been minimized.

Show comment
Hide comment
@ErisDS

ErisDS Oct 7, 2016

Member

Switched to remove the sourcemap 鉀碉笍

Member

ErisDS commented Oct 7, 2016

Switched to remove the sourcemap 鉀碉笍

@ErisDS ErisDS modified the milestone: 1.0.0-alpha.4 Oct 7, 2016

@acburdine acburdine merged commit 9edd8e3 into TryGhost:master Oct 7, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

kevinansfield added a commit to kevinansfield/Ghost-Admin that referenced this pull request Oct 21, 2016

馃悰 fix more asset paths
no issue
- TryGhost#309 introduced a change to our asset path but some images, particularly around the team page and user avatars were missed, this fixes those URLs to use the new `ghostPaths.assetRoot` helper

ErisDS added a commit that referenced this pull request Oct 21, 2016

馃悰 fix more asset paths (#360)
no issue
- #309 introduced a change to our asset path but some images, particularly around the team page and user avatars were missed, this fixes those URLs to use the new `ghostPaths.assetRoot` helper
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment