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

Removes inconsistently applied and mostly no-op double click blocking. #11719

Merged

Conversation

rtibbles
Copy link
Member

Summary

Double click blocking was originally introduced to prevent a problem seen in user navigation through the topic tree, where users would double click, resulting in a double navigation. This click blocking is no longer applied on topic navigation (there is no click mask) and yet the issue does not still seem to be prevalent.

Cleans up the click blocking logic which appears to do nothing at this juncture.

References

See original implementing PR for reference: #3915
Also fixes #7444

Reviewer guidance

I am targeting this for Planned Patch 1, as it is useful cleanup, but doesn't need to block release, and could do with a little bit of QA to ensure it doesn't introduce a regression.

The only places that might be affected is when viewing a resource in Learn, and when moving between different pages during user login/signup.

Testing checklist

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Critical user journeys are covered by Gherkin stories
  • Critical and brittle code paths are covered by unit tests

PR process

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.md

@rtibbles rtibbles added the TODO: needs review Waiting for review label Jan 11, 2024
@rtibbles rtibbles requested a review from marcellamaki January 11, 2024 23:10
@github-actions github-actions bot added APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend SIZE: small labels Jan 11, 2024
@rtibbles rtibbles force-pushed the double_click_blocking branch from 0f70277 to 4d442d6 Compare January 11, 2024 23:21
@marcellamaki marcellamaki self-assigned this Jan 16, 2024
Copy link
Member

@marcellamaki marcellamaki left a comment

Choose a reason for hiding this comment

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

With some testing, I cannot seem to find anything with navigating the tree that is not working properly as a result of this. I think it is good to merge, and if for some reason anything pops up in later QA or regression testing, we can address it.

@marcellamaki marcellamaki merged commit 91f0b3e into learningequality:release-v0.16.x Feb 27, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
APP: Learn Re: Learn App (content, quizzes, lessons, etc.) APP: User Re: User app (sign-in, sign-up, user profile, etc.) DEV: frontend SIZE: small TODO: needs review Waiting for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust the double-click blocking time to make it a little less noticeable
2 participants