Skip to content

Conversation

jkbe
Copy link
Member

@jkbe jkbe commented Jan 8, 2018

Closes #13.

  • Put the navbar links in the right order
  • Use display: table for an element in the location section so that
    the pixel height value of the section is rounded to the closest integer
    (it seems like the decimal pixel height value caused the issue with
    wrong sections being highlighted), this hinted me at the solution:
    https://stackoverflow.com/a/13103868 (decimal height value is yielded
    by bootstrap class 'img-responsive' inside location-card

- Put the navbar links in the right order
- Use display: table for an element in the location section so that
the pixel height value of the section is rounded to the closest integer
(it seems like the decimal pixel height value caused the issue with
wrong sections being highlighted), this hinted at me at the solution:
https://stackoverflow.com/a/13103868 (decimal height value is yielded
by bootstrap class 'img-responsive' inside location-card
@jkbe jkbe requested a review from flowirtz January 8, 2018 23:26
Copy link
Member

@flowirtz flowirtz left a comment

Choose a reason for hiding this comment

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

LGTM!
This introduced a bug for me however, with the 2016 ticket link. I replaced that with a static message - I think we are more than fine with that. 😊

@jkbe
Copy link
Member Author

jkbe commented Jan 9, 2018

Bug already existed before.

<!--http://eventbrite.com/tickets-external?eid=19275761321&amp;ref=etckt-->
<iframe src="../hackhpi_assets/tickets-external" frameborder="0" height="540" width="100%" vspace="0" hspace="0" marginheight="5" marginwidth="5" scrolling="auto" allowtransparency="true" style="height: 540px!important; width:100%;"></iframe>
</div>
<p class="text-center">The ticket sale is closed now.</p>
Copy link
Member Author

Choose a reason for hiding this comment

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

Does it look ok like this? Because the "col-md-12" is missing.

Copy link
Member

Choose a reason for hiding this comment

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

Looks ok on my machine!

Copy link
Member

Choose a reason for hiding this comment

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

also responsive

@flowirtz flowirtz added this to the Release milestone Jan 9, 2018
@jkbe jkbe merged commit a7cc0fd into master Jan 9, 2018
@jkbe jkbe deleted the fix-navbar-highlighting branch January 9, 2018 23:19
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.

Navigation Highlighting broken

2 participants