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

Modal anchors #76

Merged
merged 5 commits into from
Nov 6, 2018
Merged

Modal anchors #76

merged 5 commits into from
Nov 6, 2018

Conversation

marykatefain
Copy link
Contributor

There is a little bit of weirdness here where, ironically, if you go directly to the modal, although it opens and displays properly, it actually clears the hash from the URL. This is because I had to scroll to the top of the page or else the modal was out of view on load. This shouldn't really be an issue for the use case we were trying to address here.

Otherwise, if you click a card it will display the proper URL in the browser and allow users to easily share course material links.

@@ -165,4 +165,5 @@ <h3>{{course.university }}</h3>

{% block extra_js %}
<script src="{% static "js/filters.js" %}"></script>
<script src="{% static "js/modals.js" %}">></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

You've got an extra > there.

function openModal(){
var location = window.location.hash.substring(6);
var modal = "course-" + location + "-modal";
$("#" + modal).foundation("open");
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this error out when the page loads with no hash (or an invalid one)?

@marykatefain
Copy link
Contributor Author

This should be ready to merge

@marykatefain marykatefain merged commit df401c2 into master Nov 6, 2018
@marykatefain marykatefain deleted the modal-anchors branch November 6, 2018 17:59
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