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 generated event name #10932

Closed
wants to merge 1 commit into from
Closed

fix generated event name #10932

wants to merge 1 commit into from

Conversation

DanielRuf
Copy link
Contributor

As a sidenote, using the title for event names is not a great idea. We should use some variable fn-name or similar.

Copy link
Contributor

@ncoden ncoden left a comment

Choose a reason for hiding this comment

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

I don't get it. What is the link between the doc title and the event names ?

@DanielRuf
Copy link
Contributor Author

I don't get it. What is the link between the doc title and the event names ?

Well, run npm run deploy:docs and take a look at the build/drilldown-menu.html file.

@DanielRuf
Copy link
Contributor Author

@DanielRuf
Copy link
Contributor Author

The change is already done in develop but not master where the current docs live and so the docs are atill wrong.

@DanielRuf
Copy link
Contributor Author

Also see

#9151

#10555

@DanielRuf
Copy link
Contributor Author

I hereby propose to add another frontmatter variable which is used if it is set. But probably in another PR.

@JeremyEnglert
Copy link

I thought the names were generated from the inline names/comments. Such as here: https://github.com/zurb/foundation-sites/blob/1c6ce2d69e976b8d0ead214c2b2619548466c858/js/foundation.drilldown.js#L302

If this isn't the case, the recently merged PR (#10929) isn't correct.

@DanielRuf
Copy link
Contributor Author

I thought the names were generated from the inline names/comments. Such as here:

Unfortunately no.

@DanielRuf
Copy link
Contributor Author

screenshot_20180212-192608 is the current status.

@JeremyEnglert
Copy link

Eeeek. Definitely something we need to address in F7. Contributing to the docs should be one of the easier things to do.

Waiting for feedback from @ncoden, as we may have to address previous PRs as well.

@ncoden
Copy link
Contributor

ncoden commented Feb 12, 2018

I'm investigating for a solution a bit cleaner.

@ncoden
Copy link
Contributor

ncoden commented Feb 12, 2018

See foundation/foundation-docs#24
This seems resolved with that patch, so I close this PR.

@ncoden
Copy link
Contributor

ncoden commented Feb 12, 2018

See also: #10933

@DanielRuf DanielRuf deleted the patch/drilldown-menu-events-name branch February 12, 2018 21:39
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.

3 participants