Openfoodnetwork: Enable instances to add menus in the main header (with redirects)

[myriamboure]

Description

Given conversation in Slack https://openfoodnetwork.slack.com/archives/C0DNLAZC7/p1529691706000289 there is a lazy consent to do a proper implementation of "adding more menu headers with redirects" feature to avoid a hack more and more instance are doing.

Expected Behavior

Instance can add more menus headers in the top menu of an OFN website menu

Actual Behavior

Hack done for example in UK (but also Aus): See here for the code changes commit in the UK
https://github.com/openfoodfoundation/openfoodnetwork/commit/90d5a04368e229fc849a386b91d3284dd893702d
And then you need to create config/initializers/feature_toggle.rb and add the appropriate code to enable it. UK example is:

require 'open_food_network/feature_toggle'

module OpenFoodNetwork
FeatureToggle.class_eval do
private

def self.features
{order_cycles: true,
about_blog_homepage: true}
end
end
end

Context

Now France is willing to do the same so again the hack need is happening, as it's a quick fix we propose to do it properly.

Possible Fix

Add some backoffice fields that, if you fill in with URL, you get the menu on your instance.

UPDATE Luis 8/8/2018:
The feature implemented includes 7 new translation pairs (title and url) for each menu entry and then a checkbox on the Content configuration to enable/disable these menu entries. The mobile menu icons can also be defined in the content configuration (see screenshots below).

What needs to be tested:

• we need to set a few variation son the translations files and then activate/deactivate different menu entries and see if both large menu and mobile menu work properly with different type of links.

[lin-d-hop]

Please note that the existing implementations are:
UK: About, Blog, Support
Aus: Connect, Learn (they have taken About off)

This will need to work for both existing implementations.

[luisramos0]

I think the best solution here is to let each menu entry be enabled/disabled and include a URL field for the ones that can have custom URLs.
Note: the URL config fields themselves cannot be used to toggle menu because "about" and "groups", for example, are used in the footer as well. For these ones, a URL is always necessary but the entry is in the menu only for some instances.
So, this would be my suggestion for the screen :

AU config would be:

UK config would be:

Generic config would be:

FR config would be:

I am happy to do something completely different, just let me know. This is all very easy to implement. Rails+Spree at it's best really.

[myriamboure]

@mkllnk @lin-d-hop what do you think about it? What I like with that solution is that the menu headers can be translated through Transifex as well so multilingual would work. It might not answer our later needs but it does the job for now. Don't you think? Can @luisramos0 move forward with that solution? Else do you have another idea?

[Matt-Yorkley]

We need to set both the url and the link text, which is 2 fields, right? Hmmm... I'm not sure how well that would work with multilingual instances. If an instances has three languages, how would this UI work? Or do we just have a Transifex key for each link title? What if a different link was needed per language and per instance?

[lin-d-hop]

@luisramos0 @myriamboure I think that this system will also be hacked such that people can call the menu headers whatever they like in Transifex. So I wonder why specify 'About' 'Learn' 'Connect' 'Blog' 'Support' etc at all? Why not just put a maximum number of fields that users could assign whatever Label and URL they want?
Eg Repeat the following input fields 7 times
Menu Label:
URL:

My main concern with this solution is that a user could add EVERY field and that would have rendering consequences.

@Matt-Yorkley The case in which a user wants the link to link for a different URL for a different language has yet to arise. Sounds like you are over-engineering at this point to me.

[myriamboure]

@lin-d-hop I thought about it but then realized with this proposal we cannot translate the menu header, which is a problem if multilingual is enabled in your instance (not talking about the url translation...). That's why I thought Luis solution was an easy fix for our current need. Just taking the existing tabs used with their English translation, so that we can then translate them in Transifex and change the url behind them in configuration... An instance will never enable all the tabs as the app would look horrible... the instance manager will quickly realize they shouldn't do that... just looking for quick fix easy to implement. @mkllnk what do you think?

[Matt-Yorkley]

From experience I can say you'd be absolutely amazed by how horrible people will make a site look in order to squeeze random things they want into a menu that doesn't have enough space to support them.

[myriamboure]

Ok, I can hear that. So does anyone has another quick fix solution that enable us to not loose the translatability of the header menus?

[Matt-Yorkley]

I didn't mean to slow things down with my interjections, just bouncing some ideas around. UK is very keen for a solution to this, even if its a basic one.

[lin-d-hop]

I think we can think of a way to do this while being able to translate.
We could have 7 holders and label them as such:
Menu 1: [MenuTitle1]
URL Input:
Enable Checkbox:

Menu 2: [MenuTitle2]
URL Input:
Enable Checkbox:

So the [MenuTitle]s are not input fields, but in-app text which acts as a holder and can then be translated in transifex. In the en.yml file we would see:
menuTitle1: Shop
menuTitle2: Maps
menuTitle3: Producers
menuTitle4: Connect
menuTitle5: Learn

Sorry to not make mocks. I hope that makes sense.

[mkllnk]

Another quick fix idea: You can place localised views on the server to load different content: http://guides.rubyonrails.org/i18n.html#localized-views
There are four views affected though:

git grep label_connect -- app/views/
#app/views/home/_connect.html.haml:      %h2= t :label_connect
#app/views/shared/_connect_learn_call.html.haml:      = link_to t(:label_connect), "https://openfoodnetwork.org/au/connect/", target: '_blank'
#app/views/shared/menu/_large_menu.html.haml:              = t 'label_connect'
#app/views/shared/menu/_mobile_menu.html.haml:            = t 'label_connect'

The real solution will be multilingual phase 2: make database fields translatable.

[luisramos0]

Hey, thanks for the feedback!
@mkllnk I didnt know about localized views. Interesting. I am not sure I want to go that way just for this fix....
@lin-d-hop thanks for your proposal! The only advantage of my solution is that it forces people to keep the menu similar across instances.

None of the options solves the problem of translatable URLs: I just realized the translatable menus is a problem for Portugal. For example, Katuma's portuguese version cannot just point to katuma.org in Catalan as it is right now...

What if we put the URLs on the en.yml file as well @lin-d-hop ?
In the backoffice you would just enable/disable them.

We could have 7 holders and label them as such:
Enable Checkbox:
Enable Checkbox:
etc.

In the en.yml file we would see:
menuTitle1: Shop
menuURL1: /shops
menuTitle2: Maps
menuURL2: /maps
menuTitle3: About
menuURL3: /about_EN.html
etc

[myriamboure]

I'm not tech but that sounds cool @luisramos0 ! Would solve the redirections multilingual issues! What do you think @mkllnk @lin-d-hop @Matt-Yorkley ?

[Matt-Yorkley]

Yep, this suggestion gets my seal of approval.

[Matt-Yorkley]

Actually there's another issue, but maybe it's one for ofn-install. If each menu item has a checkbox to activate it, we'll have to set some defaults for which ones are checked, and also have a way for instances to set their (non-default) preferences for each one in a config file somewhere, like:

# /host_vars/openfoodnetwork.org.uk/config.yml

active_menu_items: menu1, menu2, menu4 

[mkllnk]

It's not great, but pragmatic. I wish we had time for a better solution.

[luisramos0]

@Matt-Yorkley I am not sure I understand the problem you mention. I can just set the defaults in the code. After the install (which happens once for each instance), all a instance manager has to do is to go to the backoffice and set the menu they want with the checkboxes. Right?

[sauloperez]

I agree with @mkllnk. I feel like in the rush to fix this individual problem, we're increasing a lot the complexity of the UI for something that looks more like low-level configuration to me. It feels like we were implementing WordPress: everything can be controlled by the UI and that's not the most maintainable approach.

I hope one day we can sit and go through the pros and cons and find better solutions.

[luisramos0]

I am not sure I understand your point @sauloperez.
I feel the solution is kind of simple and gives the users quite a lot of flexibility to configure their menu.

It feels like we were implementing WordPress

I think that is good! The website will be an CMS one day. It should be configurable by users/managers as much as possible.

[myriamboure]

Ok so let's move forward? It seems there is kind of a lazy (not 100% happy but still) consent? if there is still any objection please raise your voice, else let's do it!

[luisramos0]

thanks @myriamboure I think there is consent and we can move forward (I may have some time for this today) but I would like to understand what @sauloperez and @mkllnk are not happy about. If I understand, maybe I can make it more aligned with what they think.

[sauloperez]

To me, it's about the fact that we're adding a bunch of UI elements that give too much power to instances and they will have its cost in maintenance, also adding yet another way to customize things. It feels like it's something that should be code-level configuration and require a deploy to be changed but I agree on moving forward. This deserves a more in-depth discussion and evaluation.

[Matt-Yorkley]

I don't think we're "building Wordpress" here, we're cleaning up an issue that has resulted in 3 different instances using custom hacks to control their navigation links in a disorganised way (that currently complicates deployments).

There's some complexity with multilingual and different links per language, and I think we're handling that with some fairly reasonable config options.

@luisramos0 I just meant that after this feature is in place and the config options are there, we could create a corresponding part in ofn-install so that instances can set up their chosen menu configuration by defining a couple of variables in their config files instead of logging in and clicking the options manually. So for example if we reset a staging server, when we provision/deploy, those settings would be added back in automatically without extra manual steps.

[mkllnk]

I think we can move forward with this. The right solution for me would be the multilingual step 2 so that content can be translated and we don't need to put instance specific content into the locales.

The Ceres Fair Food website has a feature to pull content snippets from Wordpress. Just another idea how we can have content management without rebuilding it within the OFN. Long-term, it would be nice to integrate OFN into other websites and separate the concerns (content vs. shopping).

[luisramos0]

ok, done.

In the en.yml file:

In the Backoffice:

In the Frontoffice large menu:

In the Frontoffice mobile menu:

I decided to add the mobile menu icons to the content config, I couldn't find another better/easier/faster solution for this.

I took another little decision, let me know if you want to change: on the mobile menu, we list the available locales if there is more than one (only applicable to KATUMA today as far as I see). These languages were listed _between_ the basic menu (shops/map/producers) and the extended "feature toggled" menus. To make my life easier and the code simpler, I moved these locales to the end of all (max 7) menu entries.

Ready for code review. Let me know if you have any questions.

[daniellemoorhead]

@myriamboure @luisramos0 there was an oversight in this issue - the footer displays the top navigation links as well

The footer menu is hard coded I think. So what you have in the top menu won't reflect in the footer. Not a big issue, but worth noting just in case you want to create an issue (bug?) for this gap.

[myriamboure]

@daniellemoorhead The links behind them and related translations remain valid, am I right @luisramos0 ? I'm not sure we need to have again another same navigation in footer and header, what's the point? Shall we just remove completely the header-menu-duplicate from the footer? @RachL what is your UX thought on it? @kirstenalarsen @sstead what was the point for that decision originally? In Aus do you have a different header and footer already? I think it is the case with the hack you were doing. So I see two options:

• leave as is, so even if header menu change, leave those as footer
• remove them
  Thoughts?

[luisramos0]

Yes, the footer remains the same, it was not adapting to the menu with the previous feature toggle and now it is not adapting.
The links will always work as /shops /map /producers /groups are static. And the footer_about_url is still there on the backoffice to be configured.

[myriamboure]

It's good for me that it remains as is, was already the case, and not a priority to tackle.