-
-
Notifications
You must be signed in to change notification settings - Fork 557
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
[Article] Getting started with VoiceOver in iOS #1157
Conversation
hi @ericwbailey ! do you think you can take a look at my pull request? It's ready to be reviewed! 👍 |
Hey @Taenerys, thanks for the nudge. I'm hoping to get to it soon, just been dealing with some family stuff this week. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be a great post, thank you so much for writing it, and for your patience with the review. I have some requested changes that should hopefully not be too much to address.
The other thing I think the post could benefit from is a sentence explaining that there is also a version of VoiceOver on macOS, and that it's a separate screenreader—I know some people get tripped up over the distinction.
package.json
Outdated
@@ -46,6 +46,7 @@ | |||
"dependencies": { | |||
"@11ty/eleventy-plugin-inclusive-language": "^1.0.0", | |||
"mappy-breakpoints": "^0.2.3", | |||
"node-sass": "^5.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we revert this and changes to package-lock.json
? Sass compilation is currently handed by gulp-sass
.
Additionally, did you run into any issues getting the site to compile because of this? I want to make sure its easy to set up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes for sure! I actually was going to give a comment about this: I ran into an issue of not having node-sass package for some reasons, hence I couldn't compile and run the site the first time. I have to clear the node cache and manually add the node-sass package itself for the site to run properly. Do you have any insights on why this might happen and how I should address it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just deleted the line in package.json and looks like the website compiled fine!! Looks like it was just the first time it compiled that it happened 😅 still curious to know why though!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's super curious. Mentioning #1141 so we can reference it.
Co-authored-by: Eric Bailey <[email protected]>
Co-authored-by: Eric Bailey <[email protected]>
@ericwbailey thanks so much for your review!! I have just made a few changes based on your suggestions, and it's ready for a second review! 👍 |
@Taenerys Merged! We'll make social media announcements for the post tomorrow, and feature it in our newsletter 🎉 And thank you again for submitting this! For compensation, you can send us a message and we can talk about a suitable payment service or organization to donate to. |
This pull request addresses Issue #12 and includes the following:
Thank you!! Please let me know if there are anything I should change 👍 !