ic-labs / django-icekit

GLAMkit is a next-generation Python CMS by the Interaction Consortium, designed especially for the cultural sector.
http://glamkit.com
MIT License
47 stars 11 forks source link

Add `Navigation` model and plugins for dynamic menus #256

Closed markfinger closed 7 years ago

markfinger commented 7 years ago

re #48

@cogat I've rigged up everything except for the 'selected' state detection. I'll pick it back up on Monday, but figured you might want some input on a couple of things:

Regarding the detection of an item's selected state:

We might need to just accept that there will be edge-cases we can't predict and deal with them when they emerge. If so, I could try adding an escape hatch - eg: an optional setting pointing to a callable that takes the current path, a list of navigation items, and the default determination of the selected state for each item. It could return the selected states manipulated to whatever was appropriate for the system.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.6%) to 73.103% when pulling 86d55e165864e5160437629f3d46117a7985eb7d on feature/48-alter-the-nav-model into 4435af556fce081c7eadec72a0754b3bdbf022e1 on develop.

cogat commented 7 years ago

Can we activate it by default, please? I think it's something we'll use in every project, and would like to backport to maintained projects. And if it's used in every project, should it be a contrib?

I don't think it should do anything without a template tag to render navs, and that template tag shouldn't do anything if there's no navs defined.

I think a method on the plugin class that returns a match based on a request should work. By default, a startswith query should work. You could get the most precise one by looking at the length of the match maybe.

I think a NavItem.objects.filter(match__iregex=request.path) shouldn't be too outrageous as a fallback if we couldn't find any 'startswith' matches - it's executed natively by postgres.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.2%) to 73.947% when pulling 7a2a959567f5e06d61f7e8140b4beb9b73daa94b on feature/48-alter-the-nav-model into 23f5fffcf8b70afc8fec28295b1562000a86d85c on develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.2%) to 73.947% when pulling 7a2a959567f5e06d61f7e8140b4beb9b73daa94b on feature/48-alter-the-nav-model into 23f5fffcf8b70afc8fec28295b1562000a86d85c on develop.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 73.994% when pulling b974aeac8dc051c1deffcc5a37c1c34872f7022f on feature/48-alter-the-nav-model into 23f5fffcf8b70afc8fec28295b1562000a86d85c on develop.

markfinger commented 7 years ago

@cogat can you take a look and merge this in, if ok?

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.3%) to 74.016% when pulling f0d70171cfc7d2c0e54ed52de4c949384bde8d52 on feature/48-alter-the-nav-model into 23f5fffcf8b70afc8fec28295b1562000a86d85c on develop.

markfinger commented 7 years ago

Thanks for the feedback, Greg

cogat commented 7 years ago

No probs. Thanks for the neat solution!