Skip to content

SMTP without authentication, workaround touch API, use store theme settings for mail styling #1503

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

Merged
merged 11 commits into from
Sep 24, 2018

Conversation

rhinterndorfer
Copy link

No description provided.

Raphael Hinterndorfer added 4 commits September 13, 2018 21:45
Some SMTP servers in local networks do not need authentication to accept e-mails for relaying.
Mouse clicks to not close flyout menus when it is on a touch enabled device. Add close buttons to flyout menus.
Copy link
Contributor

@muratcakir muratcakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Placement of closer is not optimal.
  • Letter "X" is not a good choice for a closing button. Use an icon or the "×" char instead

Copy link
Contributor

@muratcakir muratcakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a good reason why we didn't do that:

  • It's not always guaranteed that every theme shares the same variables.
  • E-Mails - due to some shitty clients - require a totally different styling approach

@rhinterndorfer
Copy link
Author

What would be a better placement for a Closer?
I will check my e-Mail styling approach.

@muratcakir
Copy link
Contributor

What would be a better placement for a Closer?

It just doesn't look good in the upper tabstrip. Besides, it squashes the tab labels and cuts them in other languages with longer labels. I have no idea where to place it in a hurry.

Raphael Hinterndorfer added 2 commits September 15, 2018 12:03
Use times character instead of X character. Place closer link outside nav menu.
@rhinterndorfer
Copy link
Author

The closer now uses times character and is placed outside the nav menu. While my test I have seen that when the screen width is very small the offcanvas-cart menu uses the whole width and therefore the closer is also needed to close the offcanvas-cart without the need of reloading the site. Please check my new suggestion and if you have any ideas to make it looking better please share it with me.

Copy link
Contributor

@muratcakir muratcakir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to merge your pull request, but without the nav-closer stuff. Could you please remove it, we need a better approach for that. Actually, best way would be to fix the issue on convertible devices.

@rhinterndorfer
Copy link
Author

I have removed the closer and have done a deeper look into the smartstore.offcanvas.js script which is responsible for the menu functionality. I have changed the hiding trigger event from "tap" to "click" and now it works on all device classes (mouse, touch, mixed). I have seen on small devices where the menu uses the whole width that you can close the menu with a swipe gesture. I think this is a good solution for small devices because I think they all have touch screens.

@rhinterndorfer
Copy link
Author

Should I also revert the mail styling thing for the moment?

@muratcakir
Copy link
Contributor

Should I also revert the mail styling thing for the moment?

Yes, please.

@rhinterndorfer
Copy link
Author

I have reverted the mail styling thing.

@muratcakir muratcakir merged commit 77470a2 into smartstore:3.x Sep 24, 2018
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.

2 participants