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

Fix: Expose Adapt variable only when Dev Tools drawer is open (fixes #78) #82

Merged
merged 11 commits into from
May 14, 2024

Conversation

swashbuck
Copy link
Contributor

@swashbuck swashbuck commented Mar 21, 2024

Fixes #78

Fix

  • Exposes the Adapt variable only when the Dev Tools drawer is open
  • Adds this as a new "dev tip" in the drawer
  • Adjust styling in drawer to reduce vertical space and add formatting to keyboard commands
  • README.md - Remove required framework version and update text about Adapt variable.
  • Removed some deprecated references in js/adapt-devtools.js

Testing

  1. Open the browser console
  2. Type in Adapt into the console
  3. Open the Dev Tools drawer
  4. Type in Adapt into the console

Expected results

Adapt will only be defined when the Dev Tools drawer is open. Otherwise, you will see something like Uncaught ReferenceError: Adapt is not defined.

@swashbuck swashbuck self-assigned this Mar 21, 2024
Copy link
Contributor

@guywillis guywillis left a comment

Choose a reason for hiding this comment

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

👀

js/utils.js Outdated Show resolved Hide resolved
js/utils.js Outdated Show resolved Hide resolved
swashbuck and others added 2 commits May 6, 2024 13:59
Co-authored-by: Oliver Foster <[email protected]>
@chris-steele
Copy link
Contributor

chris-steele commented May 14, 2024

@swashbuck is it possible to have this variable when devtools is enabled, not just when the drawer is open? It'd be more useful to have the Adapt reference in the console and be able to interact with the course.

@oliverfoster
Copy link
Member

oliverfoster commented May 14, 2024

What about having it added when the dev tools is opened for the first time and not removing it thereafter? The trouble of having it enabled as a global variable whilst booting up Adapt would be best mitigated by adding the variable as late as possible and preferably not at all without intent.

@swashbuck
Copy link
Contributor Author

@swashbuck is it possible to have this variable when devtools is enabled, not just when the drawer is open?

@chris-steele This would not solve our original issue when devtools is enabled until we disable it immediately before packaging it up for release. QA would be less likely to catch any errors caused by the Adapt variable being global. In an ideal world, QA would do one final check with devtools disabled before releasing, but that rarely happens.

What about having it added when the dev tools is opened for the first time and not removing it thereafter?

@oliverfoster I think this would be fine compromise.

@swashbuck
Copy link
Contributor Author

@oliverfoster @chris-steele Let me know if you are fine with the changes in eef26dc.

@oliverfoster oliverfoster merged commit b3bdb36 into master May 14, 2024
@oliverfoster oliverfoster deleted the issue/78 branch May 14, 2024 17:22
github-actions bot pushed a commit that referenced this pull request May 14, 2024
## [3.5.1](v3.5.0...v3.5.1) (2024-05-14)

### Fix

* Expose Adapt variable only when Dev Tools drawer is open (fixes #78) (#82) ([b3bdb36](b3bdb36)), closes [#78](#78) [#82](#82)
Copy link

🎉 This PR is included in version 3.5.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change the way window.Adapt is added
4 participants