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

Update the URL when changing mailer preview formats #20983

Merged
merged 1 commit into from Oct 6, 2015

Conversation

Projects
None yet
3 participants
@jameskerr
Contributor

jameskerr commented Jul 22, 2015

Added javascript to update the URL on mailer previews with the currently selected email format. Reloading the page now keeps you on your selected format rather than going back to the default html version.

This is done using javascript's history.pushState only if the developer's browser supports it.

@arthurnn

This comment has been minimized.

Member

arthurnn commented Oct 5, 2015

pushState, I hate messing with that =).
So, what is the problem you are trying to solve? Not sure if I fully understood the description, sorry.

thanks for the PR

@jameskerr

This comment has been minimized.

Contributor

jameskerr commented Oct 5, 2015

Hi @arthurnn, thanks for reviewing this! Its my first time contributing to rails.

The problem:

When you create a mailer preview in rails, you can preview an email as the .html version or the .txt version. You do this by using that select box in the header area of the mailer preview page. When you load the mailer preview page, the select box defaults to the .html version.

screen shot 2015-10-05 at 10 20 27 am

The annoyance begins when you want to work on the .txt version of the email. Each time you make a change to the .txt email in your editor, you naturally reload your browser to see your changes. However, each time you reload, it defaults to the .html version and you are forced to re-select the .txt version from the select box.

The proposed solution:

Each time the user selects the version they want to preview, we append that mime type to the url. So, when I change the select box to show me the .txt version, I append .txt to the url. When I then reload the browser, I am brought directly to the .txt version of the email.

After creating this pull request, I realized that pushState was not the way to go. It was annoying for me to use. I have added another commit that now uses replaceState. I also noticed that the iframe was adding history when the src attribute was being changed. So I updated that to use iframe.contentWindow.location.replace(selected_format_url). This does not add any history to the browser.

So now, developers can select their desired mailer preview format, work on it, reload the page, then click back one time and go to the page they were previously on (as opposed to clicking back through all the email formats they previously selected).

PS: Let me know if I am doing something stupid in git. I feel like I didn't update my branch properly.

@arthurnn

This comment has been minimized.

Member

arthurnn commented Oct 6, 2015

@pixeltrix what do you think about this behaviour.

re: git, I think you mess it up =) . Try fetching latest rails/rails and rebase your branch against rails master.

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Oct 6, 2015

👍 to the behaviour but obviously the branch needs rebasing

@jameskerr jameskerr force-pushed the jameskerr:mailer-preview-url branch 2 times, most recently from 30d72db to baf8cd7 Oct 6, 2015

@jameskerr

This comment has been minimized.

Contributor

jameskerr commented Oct 6, 2015

@pixeltrix @arthurnn I figured out how to rebase and squash the commits. Let me know if there is any other change you would like me to make. I have been using it on a sample app, and I am enjoying the new behavior already.
out

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Oct 6, 2015

@jameskerr there are still merge conflicts preventing me from merging this - pull the latest master and then rebase against that, thanks

@pixeltrix

This comment has been minimized.

Member

pixeltrix commented Oct 6, 2015

@jameskerr also the CHANGELOG entry should be at the top of the file

@jameskerr jameskerr force-pushed the jameskerr:mailer-preview-url branch 2 times, most recently to 17ee5ab Oct 6, 2015

Update the URL when changing mailer preview formats
Added javascript to update the URL on mailer previews with the
currently selected email format. Reloading the page now keeps you on
your selected format rather than going back to the default html version.

@jameskerr jameskerr force-pushed the jameskerr:mailer-preview-url branch from 17ee5ab to 279d0ae Oct 6, 2015

@jameskerr

This comment has been minimized.

Contributor

jameskerr commented Oct 6, 2015

@pixeltrix After some trial and error, I believe I have successfully rebased with master. I put my change at the top of the changelog.

@arthurnn

This comment has been minimized.

Member

arthurnn commented Oct 6, 2015

This is great.. thanks @jameskerr . and good work. ❤️

arthurnn pushed a commit that referenced this pull request Oct 6, 2015

Arthur Nogueira Neves
Merge pull request #20983 from jameskerr/mailer-preview-url
Update the URL when changing mailer preview formats

@arthurnn arthurnn merged commit 27c970f into rails:master Oct 6, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment