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

Place the localize button above the map/below the address #8497

Merged
merged 1 commit into from
Nov 25, 2021
Merged

Place the localize button above the map/below the address #8497

merged 1 commit into from
Nov 25, 2021

Conversation

drummer83
Copy link
Contributor

What? Why?

This modification places the localize button above the map/below the address as proposed in #51.

What should we test?

The registration process works as before.

Release notes

Place the localize button above the map/below the address.

Changelog Category: User facing changes

Dependencies

None.

Documentation updates

User guides need updated screenshots (outdated anyways).
https://guide.openfoodnetwork.org/basic-features/register-and-create-your-profile

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

A screenshot in the PR description would have been nice.

@RachL RachL added the pr-staged-fr staging.coopcircuits.fr label Nov 23, 2021
@RachL
Copy link
Contributor

RachL commented Nov 23, 2021

@drummer83 thanks for this PR! Next time, process-wise I think we need to create an issue in the repo first, to validate what we want to do but also to be sure to write correctly the tests needed to validate the PR. It's also easier to assign people to the issue and avoid 2 persons working on the same bit (rare cases, but we never know!).

@filipefurtad0 I don't know what you think, but I'm guessing we will want to check different type of browsers, and check how errors message are behaving / overlap with the new position. On a standard size it seems ok, but we will need to check mobile as well.

image

I can't look at it today but if the testing column starts to be crowded, I can have a look on Thursday.

@RachL RachL removed the pr-staged-fr staging.coopcircuits.fr label Nov 23, 2021
@filipefurtad0 filipefurtad0 self-assigned this Nov 25, 2021
@filipefurtad0 filipefurtad0 added the pr-staged-uk staging.openfoodnetwork.org.uk label Nov 25, 2021
@filipefurtad0
Copy link
Contributor

Hey @drummer83 ,

Many thanks for this PR 💪

Agree with the testing approach @RachL , different devices/browsers should be checked -thanks!

The proof of concept is displayed below, for Firefox 94 (before this PR, left; after this PR right):
image

I've checked the new position of the button on:

  • Safari / iPhone
    image

  • Android/Chrome
    image

  • Android/Firefox
    image

Good to go! 🎉

@filipefurtad0 filipefurtad0 removed the pr-staged-uk staging.openfoodnetwork.org.uk label Nov 25, 2021
@jibees jibees merged commit 439f4ad into openfoodfoundation:master Nov 25, 2021
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.

6 participants