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

Some refactorings #11

Open
bwl21 opened this issue Dec 2, 2015 · 9 comments
Open

Some refactorings #11

bwl21 opened this issue Dec 2, 2015 · 9 comments

Comments

@bwl21
Copy link

bwl21 commented Dec 2, 2015

I would like to drive mosaico-php-backend to production and therefore propose some refactorings. I do not want to stick on my own, therefore I put it to discussion here.

  1. As of now image resizing is done by a php fragment included via `require( "img/resize.php" );``

    I propose to refactor resizing to a class and replace this by a method call

  2. I propose to rearrange the folder such that the root of the installation folder is kept clean:

[webroot]/[newsletter]
           dist         -- maybe mosaico
           backend      --
                dl
                upload
                img
           templates
           data
                uploads
                static
                thumbnails

I could do this refactoring if you agree and if the pull requests (#10) are resolved.

@mherbold
Copy link
Contributor

mherbold commented Dec 4, 2015

Good idea - I will look into this and possibly add a switch to editor.html to select which backend is used. I will be free to work on this project again in a couple of weeks.

@bwl21
Copy link
Author

bwl21 commented Dec 4, 2015

could you please merge the pull requests, such that I can safely continue.

@mherbold
Copy link
Contributor

mherbold commented Dec 4, 2015

I may be able to commit some time to this later today. I have to focus on my day (paying) job first. :-)

@bwl21
Copy link
Author

bwl21 commented Dec 4, 2015

cool.

Maybe we could also think of adding a microframework (slim, http://flightphp.com/ ...)

@mherbold
Copy link
Contributor

mherbold commented Dec 4, 2015

I have put everything into backend-php and also refactored stuff so it mostly runs off of one file (index.php). I also put in the absolute path fixes and I think you'll like the config.php options much better now.

@bwl21
Copy link
Author

bwl21 commented Dec 4, 2015

To be honest, I would have preferred to merge the PR and continue to refactor from there. Now I have to test everything again and have no fall back position ...

However ... please close the PRs if you have taken all out of it.

@bwl21
Copy link
Author

bwl21 commented Dec 4, 2015

I see that you have moved the configuration to an array. Cool, I already wanted to propose that.

I am not sure if your new configuration approach is able to manage the folder structure proposed in this issue. I think we should separate uploads from static and thumbnails.

@mherbold
Copy link
Contributor

mherbold commented Dec 4, 2015

Apologies about the refactor - I had actually already started the refactoring before the pull requests came in. :-\

Yes, you can separate uploads and static and thumbnail and put them in completely different places. In other words, static and thumbnails do not need to be sub-folders of the uploads folder. I just made it so by default because that is how the Mosaico default configuration is set up.

@bwl21
Copy link
Author

bwl21 commented Dec 4, 2015

I read that you cannot work a couple of weeks on this project. I am willing to contribute if it is welcome, but I don't want to waste my time.

I cannot even catch up yet because by whatever reason premailer stuff did not work on my servers.so I need back my online solution ...

I published a public email address in my profile.

bwl21 added a commit to bwl21/mosaico-php-backend that referenced this issue Dec 9, 2015
Integrated InlineStyle
Followed the layout in uSked#11

needs installation specific adaption of .htaccess
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

2 participants