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

7787 - drag to scroll on desktop #7900

Merged
merged 7 commits into from
Nov 29, 2021
Merged

7787 - drag to scroll on desktop #7900

merged 7 commits into from
Nov 29, 2021

Conversation

fessehaye
Copy link
Contributor

@fessehaye fessehaye commented Nov 20, 2021

@mofodevops mofodevops temporarily deployed to foundation-s-7787-drag-vpxuans November 20, 2021 00:32 Inactive
@fessehaye fessehaye added this to the PNI Q3 2021 milestone Nov 22, 2021
Pomax
Pomax previously requested changes Nov 22, 2021
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

The functionality to drag to scroll across subcategories are working for me across browsers and viewports, thanks Simon!

@fessehaye fessehaye requested a review from Pomax November 24, 2021 21:04
@fessehaye fessehaye temporarily deployed to foundation-s-7787-drag-vpxuans November 24, 2021 21:04 Inactive
Pomax
Pomax previously requested changes Nov 24, 2021
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
source/js/buyers-guide/search.js Outdated Show resolved Hide resolved
more cleanup
@fessehaye fessehaye temporarily deployed to foundation-s-7787-drag-vpxuans November 25, 2021 00:32 Inactive
@fessehaye fessehaye requested a review from Pomax November 25, 2021 00:33
@fessehaye fessehaye temporarily deployed to foundation-s-7787-drag-vpxuans November 25, 2021 00:36 Inactive
@Pomax
Copy link
Contributor

Pomax commented Nov 25, 2021

When testing on https://foundation-s-7787-drag-vpxuans.herokuapp.com/en/privacynotincluded/categories/smart-thermostats-smoke-detectors/ It looks like this works once (I can click and drag to move the subcats around) but after a mouse release/mouse up it stops working.

@Pomax
Copy link
Contributor

Pomax commented Nov 25, 2021

it seems to not always work though, sometimes on a page load nothing seems to happen (other than text-selecting)

Pomax
Pomax previously requested changes Nov 25, 2021
Copy link
Contributor

@Pomax Pomax left a comment

Choose a reason for hiding this comment

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

marking as "request changes" for now based on the events not reliably firing

@fessehaye fessehaye temporarily deployed to foundation-s-7787-drag-vpxuans November 25, 2021 17:51 Inactive
@fessehaye fessehaye requested a review from Pomax November 25, 2021 17:58
@Pomax
Copy link
Contributor

Pomax commented Nov 29, 2021

It looks like the highlighted subcategory doesn't quite make it into view all the time, but if @sabrinang's okay with it we can land this first, then fix that in a follow-up issue.

image

@Pomax Pomax temporarily deployed to foundation-s-7787-drag-vpxuans November 29, 2021 19:42 Inactive
@Pomax Pomax requested a review from nancyt1 November 29, 2021 19:56
@Pomax
Copy link
Contributor

Pomax commented Nov 29, 2021

@nancyt1 could you have a look and see if more work is required (although not necessarily as part of this PR itself)?

Copy link
Collaborator

@nancyt1 nancyt1 left a comment

Choose a reason for hiding this comment

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

Drag to scroll works for me across viewports and selected subcategories snapped into viewport when clicked for me. I think it looks good!

(Not sure if it's just the review app but drag to scroll doesn't work after I refresh the page, only when I first land on it)

@Pomax Pomax temporarily deployed to foundation-s-7787-drag-vpxuans November 29, 2021 22:17 Inactive
@fessehaye fessehaye temporarily deployed to foundation-s-7787-drag-vpxuans November 29, 2021 22:58 Inactive
@fessehaye fessehaye merged commit 8eff4f4 into main Nov 29, 2021
@fessehaye fessehaye deleted the 7787-drag branch November 29, 2021 23:14
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.

[PNI QA] Subcategories — drag to scroll on desktop
5 participants