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

Rework DOM to better separate elements from events #1074

Merged
merged 9 commits into from
Oct 29, 2024

Conversation

illicitonion
Copy link
Member

What does this change?

Fixes #332

This way, we:

  • Fully handle accessing and setting DOM element properties in one chunk, then introduce events, rather than muddling the two together.
  • Identify that there are two common actions to do, and talk through identifying and addressing this.
  • Pull out the "what is the character limit" to be defined by the HTML attribute, rather than duplicated in the script.
  • Practice breaking down the "remaining characters" problem into sub-problems.
  • Generally solve things more incrementally.
  • Explicitly call out "do a clean-up refactoring" at the end.

Also some misc copy edits.

Common Content?

Yes - changes, reorders, and adds blocks to much of JS2.

Org Content?

Data Groups Sprint 3

Checklist

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-itd ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-itd/deploys/6720f0ad208c570008b8685c
😎 Deploy Preview https://deploy-preview-1074--cyf-itd.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 92 (🔴 down 8 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-programming ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-programming/deploys/6720f0ad79253300086bdf2f
😎 Deploy Preview https://deploy-preview-1074--cyf-programming.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 85 (🔴 down 14 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 90 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-sdc ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-sdc/deploys/6720f0ad853f5400086f4e66
😎 Deploy Preview https://deploy-preview-1074--cyf-sdc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-curriculum canceled.

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-curriculum/deploys/6720f0adb406d70008555674

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-tracks ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-tracks/deploys/6720f0ad3b56780008cacd7f
😎 Deploy Preview https://deploy-preview-1074--cyf-tracks.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-launch ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-launch/deploys/6720f0ad6decc00008bceb14
😎 Deploy Preview https://deploy-preview-1074--cyf-launch.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-piscine ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-piscine/deploys/6720f0ad3c94ab00085a90cb
😎 Deploy Preview https://deploy-preview-1074--cyf-piscine.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (🟢 up 4 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 100 (no change from production)
PWA: 60 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cyf-common ready!

Name Link
🔨 Latest commit bb57bdc
🔍 Latest deploy log https://app.netlify.com/sites/cyf-common/deploys/6720f0ad18c79c00089a104c
😎 Deploy Preview https://deploy-preview-1074--cyf-common.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@SallyMcGrath SallyMcGrath left a comment

Choose a reason for hiding this comment

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

tiny notes

common-content/en/module/js2/events/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/events/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/events/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/plan/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/plan/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/querying/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/querying/index.md Outdated Show resolved Hide resolved
common-content/en/module/js2/querying/index.md Outdated Show resolved Hide resolved
illicitonion and others added 9 commits October 29, 2024 14:26
Fixes #332

This way, we:
* Fully handle accessing and setting DOM element properties in one
  chunk, then introduce events, rather than muddling the two together.
* Identify that there are two common actions to do, and talk through
  identifying and addressing this.
* Pull out the "what is the character limit" to be defined by the HTML
  attribute, rather than duplicated in the script.
* Practice breaking down the "remaining characters" problem into
  sub-problems.
* Generally solve things more incrementally.
* Explicitly call out "do a clean-up refactoring" at the end.

Also some misc copy edits.
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
Co-authored-by: Sally McGrath <sally@codeyourfuture.io>
@illicitonion illicitonion enabled auto-merge (squash) October 29, 2024 14:27
@illicitonion illicitonion merged commit 962ecdf into main Oct 29, 2024
32 of 34 checks passed
@illicitonion illicitonion deleted the dwh-dg-sprint3-rework branch October 29, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Rework JS2W3 prep to introduce "setting a DOM property" _before_ event handlers
2 participants