Skip to content

Conversation

othree
Copy link
Contributor

@othree othree commented Jun 7, 2019

Fixes #146

Didn't call t.click() because iOS might disable all these kind of operations(js to mimic user interaction) in the future.

@othree
Copy link
Contributor Author

othree commented Jun 7, 2019

Also fix the smooth scroll behavior of menu link.

Copy link
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

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

This code change makes sense to me and simplifies a bit. Do we have a sense of how compatible it is across browsers?

Copy link
Member

@littledan littledan left a comment

Choose a reason for hiding this comment

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

This code change makes sense to me and simplifies a bit. Do we have a sense of how compatible it is across browsers?

@littledan
Copy link
Member

@bkardell, could you review this?

@codehag
Copy link
Collaborator

codehag commented Jul 23, 2019

@othree - this looks good to me, can you rebase?

othree added 2 commits July 24, 2019 11:56
Use css instead of JS to scroll to target
Which use less code and can contain navigation history, also make the
url in location bar correct.
@othree
Copy link
Contributor Author

othree commented Jul 24, 2019

@codehag I have updated the PR. But the CI failed for unknown reason.

@othree
Copy link
Contributor Author

othree commented Jul 25, 2019

I found its prettier check failed issue.
But the message is not clear.

Should we run prettier manually?
Or use pre-commit hook to handle it(but I saw lint-staged and husky are removed).
Not sure which one is prefered solution.

@ljharb
Copy link
Member

ljharb commented Jul 25, 2019

generally one runs linting/formatting manually.

@codehag
Copy link
Collaborator

codehag commented Jul 30, 2019

This looks good now, thanks.

Regarding the prettier question, we can bring it up in the website meeting.

@codehag codehag merged commit 129fc7e into tc39:master Jul 30, 2019
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.

mobile menu toggling

4 participants