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

Assorted tweaks and updates #173

Merged
merged 19 commits into from
Apr 17, 2021
Merged

Assorted tweaks and updates #173

merged 19 commits into from
Apr 17, 2021

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Apr 5, 2021

@bryanbraun Please don't squash this and don't let it hanging for a long time :)

Let me know if you need me to make any changes.

Notes:

  • I didn't update the dist files, etc, but after merging this, a new patch release would be perfect.
  • Ideally, we should have karma run for the minified file just to be 100% safe, but that's for another time (we should have an issue to track it)
  • We could add coverage support later

Manual Preview: https://sleepy-noyce-bed069.netlify.app


for(var i = 0; i < toggleButtons.length; i++) {
toggleButtons[i].addEventListener("click", slideExampleCode); // add the event to each button
for (var i = 0; i < toggleButtons.length; i++) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could probably use one variable, but I went with the way I'd go if this was a let variable. Feel free to change it later :)

@XhmikosR XhmikosR marked this pull request as ready for review April 5, 2021 05:51
@@ -39,7 +39,6 @@
</head>
<body>
<section class="main">
<a href="https://github.com/bryanbraun/anchorjs"><img style="position: absolute; top: 0; right: 0; border: 0;" src="https://github-camo.global.ssl.fastly.net/365986a132ccd6a44c23a9169022c0b5c890c387/68747470733a2f2f73332e616d617a6f6e6177732e636f6d2f6769746875622f726962626f6e732f666f726b6d655f72696768745f7265645f6161303030302e706e67" alt="Fork me on GitHub" data-canonical-src="https://s3.amazonaws.com/github/ribbons/forkme_right_red_aa0000.png"></a>
<h1 class="page-title">AnchorJS</h1>
<div class="anchorjs-logo"></div>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could probably remove this too or use the SVG file, but I thought I'd leave this to you :)

@@ -32,7 +32,7 @@
<body>
<section class="main">
<h1>A page with large images (delays after DOMContentLoaded).</h1>
<img src="http://lorempixel.com/1600/1600/" alt="Large image">
<img src="https://loremflickr.com/1600/1600/" alt="Large image">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't get a 1600x1600 image, though, but I don't have a replacement in mind. Feel free to adapt this later :)

Copy link
Owner

Choose a reason for hiding this comment

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

Gotcha. This is definitely an improvement.

@XhmikosR XhmikosR changed the title Dev patches Assorted tweaks and updates Apr 5, 2021
@@ -244,8 +244,7 @@

// Regex for finding the non-safe URL characters (many need escaping):
// & +$,:;=?@"#{}|^~[`%!'<>]./()*\ (newlines, tabs, backspace, vertical tabs, and non-breaking space)
var nonsafeChars = /[& +$,:;=?@"#{}|^~[`%!'<>\]./()*\\\n\t\b\v\u00A0]/g,
urlText;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can revert the temp var removal; I thought it helped with uglify but apparently it doesn't. So, if it's more clear for you with the temp var, let me know and I'll revert it.

Copy link
Owner

Choose a reason for hiding this comment

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

I think it's good without the temp variable. 👍

So that we are consistent and that it's more readable.
@@ -19,7 +19,7 @@
<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Source+Sans+Pro:200,400,700&display=swap">
<link rel="stylesheet" href="styles.css">
<link rel="stylesheet" href="fonts/fonts.css">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/prismjs@1.21.0/themes/prism-twilight.css">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/prismjs@1.23.0/themes/prism-twilight.css">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This update fixes IE 11 errors among other things

Copy link
Owner

@bryanbraun bryanbraun left a comment

Choose a reason for hiding this comment

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

These are really great changes all around! Thanks for your patience waiting until I could get to it. I'll merge it and roll out a patch update shortly. 👍

@@ -244,8 +244,7 @@

// Regex for finding the non-safe URL characters (many need escaping):
// & +$,:;=?@"#{}|^~[`%!'<>]./()*\ (newlines, tabs, backspace, vertical tabs, and non-breaking space)
var nonsafeChars = /[& +$,:;=?@"#{}|^~[`%!'<>\]./()*\\\n\t\b\v\u00A0]/g,
urlText;
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's good without the temp variable. 👍

@@ -32,7 +32,7 @@
<body>
<section class="main">
<h1>A page with large images (delays after DOMContentLoaded).</h1>
<img src="http://lorempixel.com/1600/1600/" alt="Large image">
<img src="https://loremflickr.com/1600/1600/" alt="Large image">
Copy link
Owner

Choose a reason for hiding this comment

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

Gotcha. This is definitely an improvement.

@bryanbraun bryanbraun merged commit 8961d72 into bryanbraun:master Apr 17, 2021
@XhmikosR XhmikosR deleted the dev branch April 18, 2021 05:29
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.

2 participants