-
Notifications
You must be signed in to change notification settings - Fork 32
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
Block out Menus #56
Block out Menus #56
Conversation
Laying down some basic menu structure for inspiration. Open to change, just helping to define the rough shape of things.
…ering out menu items based on event state (active/over), but for now I am leaving everything in for clarity.
</head> | ||
<body> | ||
@{ | ||
var Event = ViewData["Event"] as ServerCore.DataModel.Event; |
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.
Is it necessary to use ViewData in layouts or is there some way to reference the model?
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.
The book I have says ViewData is needed for things in the shared file. With more engineering I am sure there is something more strongly typed but I am not sure it is worth it. I do have plans to automatically fill in the Event via routing and other things, which will help somewhat.
|
||
@if (Event != null) | ||
{ | ||
<li class="dropdown"> |
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.
Should we indent from the curly brace @asyasky?
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 please. :) An automatic style sheet is on my todo list, but in the mean time ctrl+k+d fixes up most things (not sure about cshtml, but it does in cs for sure).
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.
Looks like a good start. For future significant UI changes, a screenshot might be nice to make it a bit easier to review (you can just drag/paste them into the description)
Screenshots are a bit of a chore with dropdowns but in general I get your point :-) |
And thanks for the review! |
Laying down some basic menu structure for inspiration. Open to change,
just helping to define the rough shape of things.
Linked to Issue #53.