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

feat: store locator #911

Merged
merged 2 commits into from Feb 7, 2022
Merged

feat: store locator #911

merged 2 commits into from Feb 7, 2022

Conversation

shauke
Copy link
Member

@shauke shauke commented Oct 22, 2021

PR Type

[x] Feature

What Is the Current Behavior?

Currently the PWA does not provide Store Finder functionality similar to the one in the Responsive Starter Store
https://pwa-review.northeurope.cloudapp.azure.com/INTERSHOP/web/WFS/inSPIRED-inTRONICS_Business-Site/en_US/-/USD/ViewStoreFinder-Start

Issue Number: Closes #754

What Is the New Behavior?

Store Finder functionality is available and configurable via feature toogle.
In addition Google Maps integration can be activated.

Store Finder with Google Maps integration is activated by default for the B2B theme
http://pwa-gh-review-911.azurewebsites.net/store-finder

Store Finder without Google Maps integration is the default configuration for the B2C theme
http://pwa-gh-review-911.azurewebsites.net/store-finder;theme=b2c

Does this PR Introduce a Breaking Change?

[x] No

Other Information

AB#70748

To Dos

  • IAD/VD review
  • Store Finder or Store Locator link integration
  • code review
  • documentation
  • waiting for REST API "'stores' resource should return custom attributes 'Latitude' and 'Longitude'"
  • remove mock data and docker ignore adaption
  • remove maps key from env
  • scroll behavior parent
  • rebase for git credit
  • handle "no results" response
  • update linting to ESLint
  • api key as env var
  • ability to provide highlight icon
  • fix tests

@shauke shauke added the feature New feature or request label Oct 25, 2021
@shauke shauke assigned shauke and MoritzRS and unassigned shauke Oct 25, 2021
Copy link

@iwiederhold iwiederhold left a comment

Choose a reason for hiding this comment

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

store_finder1

Here are the paddings for the new footer-top:
div class="footer-top" style="
padding-top: 20px;
padding-bottom: 20px;"

store_finder2

@MoritzRS
Copy link
Contributor

The Google Maps API does allow customization and styling. It is possible to allow or disallow controlls (zoom, map type, street view, etc.), set min and max zoom level of the map, place custom markers on the map by providing an image or even stylizing the map itself.

Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

I like the google maps integration 👍 Nice feature!

We would like to change a little bit the user experience design. Here are the IAD/VD remarks:

  • Small text changes in Store finder introduction (see designs below).
  • Space between paragraph and store finder form is too large. I think it should be 15px between the explanation "Simply select a country..." and the form (see registration form as example).
  • Typo: ZIP/Postal Code (see labels in registration form - each word starts with capital letters, no blank character after /)
  • "Results of store location finder" heading is no longer displayed - only a counter for the results (paragraph like "no-search-result-title").
  • Display a list of stores on the left side (with scrollbar) and map is displayed on right side.
  • If user hovers over a list item (background-color: #e6e6e6), the pin is highlighted (highlight definition is in progress). Triggered by click on mobile devices.
  • Please display the e-mail in the store contact information.
  • Use icons for phone, fax and e-mail (fa-phone, fa-fax, fa-envelope).
  • Phone and e-mail are links (tel: and mailto:) so that users can contact the store immediately without copying the number or e-mail address.
  • Phone view: Display store list below map (see designs below).

Desktop:
Map
Phone:
Phone-Map
No map:
List
No results:
Empty

@iwiederhold
Copy link

Customizing the map marker:

You can change the pins to another images
shoppingcart_pinlet-2-medium
for all stores that have been found

spotlight_pin_v4_dot-1-small
for the selected store (hover and selected)

You can find these icons in Google Maps.
shoppingcart_pinlet-2-medium.png&highlight=4285f4,5491f5,ffffff
spotlight_pin_v4_dot-1-small.png&highlight=c5221f,ea4335,b31412

iwiederhold
iwiederhold previously approved these changes Nov 9, 2021
Copy link

@iwiederhold iwiederhold left a comment

Choose a reason for hiding this comment

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

Great work! THX

Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

It looks fantastic. I would like to change one last things:

  • Use the default margin of 16px bottom for paragraph text. I think the PWA uses only "p" (see contact form) or "p class=ng-star-inserted" (see my account pages) most of the cases. Your paragraph uses class="my-4" with 24px margin top and bottom (see screenshot below). @MoritzRS: Why you use this class? Is it something special?
  • Please use the solid envelope as icon for e-mail (i class="fas fa-envelope"). I used the false icon for the design. Sorry, this was my mistake.
  • If you click on a pin on the map, then the list should move to the selected item on the left side.
  • Please change the selection of a list item from hover to click on desktop view. During the review, the behavior feels unsteady and the user losses control (example: I scroll through the list and then the selected pin is changed without me wanting it). It feels much better on the current behavior in mobile view. That's why I decided to make this change.

2021-11-09 11_33_40-Intershop Progressive Web App _ Intershop PWA

@MoritzRS
Copy link
Contributor

Scrolling itself should be supported on all modern browsers (uses Element.scrollTo). However smoothing via option.behavior might not be supported on every browser. Works fine on Chrome and Firefox but couldn't use smooth on Epiphany (WebKit based) so Safari might also not be working as well. Depending on the use case, polyfills might be needed.

@MoritzRS MoritzRS requested a review from M-Behr November 10, 2021 08:15
Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

It looks and works very well.
Currently the list items look not clickable.
2021-11-10 09_26_53-Intershop Progressive Web App _ Intershop PWA

Therefore we have to add 2 little things:

  • The hand as mouse pointer, which indicates an action. You can see it when you hover over buttons, links or pins.
  • And a hover styling is needed (background color e6e6e6).

These are the last changes, I promise! :)

@MoritzRS MoritzRS requested a review from M-Behr November 10, 2021 10:09
M-Behr
M-Behr previously approved these changes Nov 10, 2021
Copy link

@M-Behr M-Behr left a comment

Choose a reason for hiding this comment

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

Great job 👍

@MaxKless MaxKless assigned MaxKless and unassigned MoritzRS Jan 31, 2022
@MaxKless MaxKless changed the base branch from develop to feature/scroll-to-error-message February 1, 2022 14:12
@MaxKless MaxKless force-pushed the feature/scroll-to-error-message branch 2 times, most recently from 755e19e to 37d58b0 Compare February 1, 2022 14:23
Base automatically changed from feature/scroll-to-error-message to develop February 1, 2022 15:51
@MaxKless MaxKless marked this pull request as ready for review February 3, 2022 14:28
@M-Behr M-Behr self-requested a review February 4, 2022 08:49
M-Behr
M-Behr previously approved these changes Feb 4, 2022
MoritzRS and others added 2 commits February 7, 2022 14:14
Co-authored-by: max.kless@googlemail.com <max.kless@googlemail.com>
Co-authored-by: Stefan Hauke <s.hauke@intershop.de>
Co-authored-by: Silke <s.grueber@intershop.de>
@shauke shauke merged commit 9a433e9 into develop Feb 7, 2022
@shauke shauke deleted the feature/store-locator branch February 7, 2022 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Store locator missing on PWA
5 participants