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

Remove CDN & Implement in-use css from CDN into internal css #625

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

DhananjayPurohit
Copy link

@DhananjayPurohit DhananjayPurohit commented Aug 27, 2021

Created this pull request in response to issue #517. Removed CDN from all the files serving UI for SentEmailViewer and implemented css being in use from CDN into internal css.

@germsvel
Copy link
Collaborator

@DhananjayPurohit thanks for this contribution!

Since we're replacing normalize, and that could potentially change the styling of the email previewer, could you include before and after screenshots for any major browsers like Chrome, Firefox, Safari, and Edge? It's tough to review the PR without being able to see how things change.

@DhananjayPurohit
Copy link
Author

Thanks @germsvel for giving it a review!

Attaching the screenshots taken while running on Chrome along with the inspect window.

Before making changes:-

Screenshot (298)
Screenshot (299)

After making changes:-

Screenshot (300)
Screenshot (301)

As you can see the inspect window having CDN before making any changes and replacing the CDN on making changes. I would love to test on other browsers too if there is any possibility of breaking on any browser.

@seivan
Copy link

seivan commented Oct 28, 2021

@germsvel @DhananjayPurohit I am not a CSS expert, but I have a few questions that might help here.

First off, is the idea of using SentEmailViewer to give an impression of how the email will look like on the receivers end or just make it palatable for the developer monitoring it?

Because if it's for the former, it's making the assumption that the receiver of said email will have normalize on their client or the email itself will contain it when sent.

Why optimize with a separate stylesheet or CDN since this is a developer plugin and most likely not a front facing feature in other words: is it possible to just embed the content of normalize.css into the HTML directly?

@seivan
Copy link

seivan commented Oct 28, 2021

@DhananjayPurohit This doesn't implement everything from normalize.css

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.

None yet

3 participants