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

Drag level and scroll #37965

Merged
merged 7 commits into from Nov 24, 2020
Merged

Drag level and scroll #37965

merged 7 commits into from Nov 24, 2020

Conversation

davidsbailey
Copy link
Member

@davidsbailey davidsbailey commented Nov 21, 2020

Background

Starts PLAT-430. Previously, #37384 moved the sectionMetrics state up to ActivitiesEditor and made sure to update so that dragging would work when the page was scrolled before dragging starts. This PR deals with a related problem of making sure that dragging will work when scrolling happens after dragging starts.

This PR tackles two specific problems:

  1. "drop" target does not follow the cursor (I had to use a mouse with a scrollwheel to capture this behavior)
    drag-wrong-drop-target

  2. dragging doesn't cause the page to scroll
    drag-no-scroll

Description

Here's what this PR does:

  • af741dc "call updateActivitySectionMetrics on scroll while level is being dragged". this recomputes the position (clientY) of the drop targets / ActivitySectionCards within the window, which become stale after scrolling, fixing problem 1.
  • 381297b explicitly clear targetActivitySection. This is a minor fix to un-highlight the drop target after the LevelToken is dropped.
  • the rest of the commits cause the page to scroll progressively faster when dragging within 100px of the top or bottom of the window (problem 2).

Here is how it looks:
scroll-and-drop-working

Future work

The following problems are tackled in the next PR:
3. the dragged element does not follow the cursor
4. the logic for reordering levels within the original activity section is broken

Testing story

Manual testing only for drag and drop behavior. Existing test coverage guards against regressions in other features within the lesson editor.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

@davidsbailey davidsbailey changed the base branch from staging to import-section-position November 23, 2020 19:32
Base automatically changed from import-section-position to staging November 23, 2020 20:43
@davidsbailey davidsbailey marked this pull request as ready for review November 23, 2020 21:31
Copy link
Contributor

@bethanyaconnor bethanyaconnor left a comment

Choose a reason for hiding this comment

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

Tested locally and looks good. Code makes sense so lgtm!

@davidsbailey davidsbailey merged commit a135c8f into staging Nov 24, 2020
@davidsbailey davidsbailey deleted the drag-level-and-scroll branch November 24, 2020 00:30
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.

None yet

2 participants