-
-
Notifications
You must be signed in to change notification settings - Fork 148
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
feat(nav): Add z-index on the 'Navigate up' section #23
Conversation
It is nice to have a link to the z-index somewhere, but having it in
the "Navigate up" section doesn't feel right to me. Maybe it would be
make more sense for it to be in the footer?
|
@humaidq I didn't find a place to put it that wouldn't require bigger layout/markup changes. IMHO the graph is the top level so I find it intuitive to be part of 'Navigate up'. That said, I'm fine with it being anywhere in the footer. There's an open issue for 'Edit on GitHub' (#5), maybe the footer will be extended with more things and a better place from UX point of view. |
I like the idea of including "z-index" in a place like this. I agree that it feels odd to place it in "Navigate up" section (though that would eventually make sense once "z-index" becomes part of the graph itself, as a result of #18). For now, I'd introduce a separate footer section below both Connections/NavigateUp (still inside the same teal box perhaps) and put the z-index in there. |
src/Neuron/Zettelkasten/View.hs
Outdated
li_ $ do | ||
a_ [class_ "zettel-link item", href_ "/z-index.html", title_ "z-index"] $ do | ||
span_ [class_ "zettel-link-title"] "z-index" |
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.
Respecting the DRY principle, I'd extract this code into a function, and then re-use it here.
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.
Extracted the two links, active and regular. Although active is not a link is used in place of one.
They aren't exactly the same to justify using the same function I think.
Also render z-index link in separate section
@gillchristian I've made some tweaks to your branch at gillchristian#1 ... if you could merge that PR, I'll merge this one right away. |
Actually, I've merged it from the command line. Thanks for the PR! |
This PR adds a link to the
z-index
in the "Navigate up" section as a quick way to go back to the graph view.NOTE: I'm not familiar with Rib and the Nueron codebase. Please let me know what should be changed.