From c53cfc595109d97909716ef2ae0b7fed44687be0 Mon Sep 17 00:00:00 2001 From: Ridhwana Date: Tue, 23 Mar 2021 11:12:54 +0200 Subject: [PATCH] RFC#50-P4: Feature Flagged Routes and Interaction Design (#12967) * feat: add the nested sidebar with some elements * feat: create a tabbed nav item menu * feat: add the tabbed nav_item partial to the views that need tabbed nav items * fix: change variable back * feat: style the sidebar a bit more * chore: add some more styles * feat: add a spec for the nested navigational items * refactor: a more dynamic tabbed admin helper * feat: add some more nav items * fix: controller for reports * refactor: shorthand if statement * chore: add the whitespace back * refactor: rubocop fixes * chore: use any * chore: remove whitespace * refactor: rename the variable * refactor: use a DSL style admin helper * chore: variable renaming and routes * rubocop: fixes * refactor: move files to more apt places * chore: keep overview as it was previously * Update app/views/admin/secrets/index.html.erb Co-authored-by: Michael Kohl * Update app/views/admin/shared/_tabbed_navbar.erb Co-authored-by: Michael Kohl * Update app/views/admin/badges/index.html.erb Co-authored-by: Michael Kohl * chore: disable blocklength * refactor: move the logic to the model instead of in the view * chore: remove get_ prefix * chore: move the request mangling to a helper that finds the controller and scope * Update app/helpers/admin_helper.rb Co-authored-by: rhymes * Update app/helpers/admin_helper.rb Co-authored-by: rhymes * refactor: Address feedback * oops * oops use tr * feat: update specs * feat: make the navbar a dropdown * feat: add a cursor pointer to the dropdown * feat: add the icons which results in changed data structure * fix: badge achievements * feat: rename to an html file, show and collapse links + show active links * chore: rename tabbed view to an html file * fix: scope should be apps not app * feat: add icons for the admin menu * feat: increase the margin left * feat: move the overview into the feature flag block and add an icon * chore:remove files * chore: indent * feat: update crayons -link to have no text-decoration * feat: current link for a scope with one controller * Update app/lib/menu.rb Co-authored-by: Michael Kohl * Update app/models/admin_menu.rb Co-authored-by: Michael Kohl * refactor: we added svg to the builder so remove it from creating the hash * feat: undo change to crayons and add it to the admin stylesheet * Update app/views/admin/shared/_nested_sidebar.html.erb Co-authored-by: Suzanne Aitchison * refactor: change to use ul and li's + a button * chore: add bracket to next line * feat: add aria-page * chore: remove brackets * feat: added focus specifically for the sidebar * Update app/views/admin/shared/_nested_sidebar.html.erb Co-authored-by: Jamie Gaskins * chore: remove additional title * chore: indent * feat: add a visibilee keyword to the payload and set it to true by default * feat: move the feature flagged routes into the correct sections * feat: check if an item is visible before rendering it * feat: amend the tabbed_navbar to be more accessible and add in visibilty checks * chore: update comment * chore: amend the styles * chore: change url to path * chore: comment * test: add more tests * Update app/assets/stylesheets/admin.scss Co-authored-by: Suzanne Aitchison * Update app/views/admin/shared/_nested_sidebar.html.erb Co-authored-by: Suzanne Aitchison * Update app/views/layouts/admin.html.erb Co-authored-by: Suzanne Aitchison * chore: merge * feat: use focus for browsers that dont support focus-visible (I'm looking at you Safari) and move it within the crayons-link so we dont see it on mouse click * WIP: first pass of stimulus controller code * feat: interaction design * chore: remove unnecessary condiition * chore: only add transparent background when not the current link * Update app/models/admin_menu.rb Co-authored-by: Michael Kohl * feat; comment explaining * feat: add an id on the button to be clicked * feat: disable currentNavItem * chore: remove event params * chore: update cursor * feat: (safe fail) only show the tabbed navbar when the roures contain values form the data structure * chore: pass events through + tests * trigger an onload event and test the disabling of the menu item * refactor: tabbed menu items * feat: account for the visibility of the feature flags * chore: rubocop fixes * chore: indentation * feat: some refactors and updates for rubocop * Update app/javascript/admin/controllers/sidebar_controller.js Co-authored-by: Vaidehi Joshi * feat: set to true Co-authored-by: Michael Kohl Co-authored-by: rhymes Co-authored-by: Suzanne Aitchison Co-authored-by: Jamie Gaskins Co-authored-by: Vaidehi Joshi --- app/assets/stylesheets/admin.scss | 33 +++-- .../controllers/sidebar_controller.test.js | 122 ++++++++++++++++++ .../admin/controllers/sidebar_controller.js | 35 +++++ app/lib/menu.rb | 4 +- app/models/admin_menu.rb | 39 +++++- .../admin/shared/_nested_sidebar.html.erb | 42 +++--- .../admin/shared/_tabbed_navbar.html.erb | 30 +++-- app/views/layouts/admin.html.erb | 6 +- config/routes.rb | 52 +++++--- spec/requests/admin/nested_sidebar_spec.rb | 50 ++++++- 10 files changed, 343 insertions(+), 70 deletions(-) create mode 100644 app/javascript/admin/__tests__/controllers/sidebar_controller.test.js create mode 100644 app/javascript/admin/controllers/sidebar_controller.js diff --git a/app/assets/stylesheets/admin.scss b/app/assets/stylesheets/admin.scss index 22a8bd163..c95d7afca 100644 --- a/app/assets/stylesheets/admin.scss +++ b/app/assets/stylesheets/admin.scss @@ -189,11 +189,29 @@ label { min-width: 120px; } -// Navbar links have text-decorations that are being -// rendered from boostrap. We want to remove these to be -// consistent with links in the crayons design system. .admin__left-sidebar { - .crayons-link { + button { + border: 0; + width: 100%; + + // In safari the links show up with a grey background + &:not(.crayons-link--current) { + background: transparent; + } + } + + .crayons-link--current { + cursor: default; + } +} + +.admin__tabbed-navbar, +.admin__left-sidebar { + // Navbar links have text-decorations that are being + // rendered from boostrap. We want to remove these to be + // consistent with links in the crayons design system. + .crayons-link, + .crayons-tabs__item { text-decoration: none; &:hover { @@ -213,15 +231,8 @@ label { } } - button { - border: 0; - width: 100%; - background: transparent; - } - ul { list-style: none; padding: 0; } - } diff --git a/app/javascript/admin/__tests__/controllers/sidebar_controller.test.js b/app/javascript/admin/__tests__/controllers/sidebar_controller.test.js new file mode 100644 index 000000000..f42f40258 --- /dev/null +++ b/app/javascript/admin/__tests__/controllers/sidebar_controller.test.js @@ -0,0 +1,122 @@ +import { Application } from 'stimulus'; +import SidebarController from '../../controllers/sidebar_controller'; + +describe('SidebarController', () => { + beforeAll(() => { + document.head.innerHTML = + ''; + }); + + beforeEach(() => { + document.body.innerHTML = ` +
+ +
`; + + const application = Application.start(); + application.register('sidebar', SidebarController); + }); + + describe('#disableCurrentNavItem', () => { + it('sets the disabled attribute on the open menu button', () => { + window.dispatchEvent(new Event('load')) + const button = document.getElementById('apps_button'); + + expect(button.getAttribute("disabled")).toEqual("true"); + }); + }); + + describe('#expandDropdown', () => { + beforeEach(() => { + let assignMock = jest.fn(); + + delete window.location; + window.location = { href: assignMock }; + }); + + afterEach(() => { + window.location = location; + }); + + it('redirects to the first child navigation item', () => { + const button = document.getElementById('advanced_button'); + button.click(); + + expect(window.location.href).toEqual("/admin/advanced/broadcasts") + }); + + it('closes other menu items', () => { + const button = document.getElementById('advanced_button'); + button.click(); + + expect(document.getElementById('apps').classList).toContain("hide"); + }); + + }) +}); diff --git a/app/javascript/admin/controllers/sidebar_controller.js b/app/javascript/admin/controllers/sidebar_controller.js new file mode 100644 index 000000000..ddbd01caf --- /dev/null +++ b/app/javascript/admin/controllers/sidebar_controller.js @@ -0,0 +1,35 @@ +import { Controller } from 'stimulus'; + +// eslint-disable-next-line no-restricted-syntax +export default class SidebarController extends Controller { + static targets = [ + 'submenu' + ]; + + disableCurrentNavItem() { + const activeMenuId = this.submenuTargets.filter((item) => item.classList.contains("show"))[0].id + const activeButton = document.getElementById(`${activeMenuId}_button`); + activeButton.setAttribute("disabled", true) + } + + expandDropdown(event) { + this.redirectToFirstChildNavItem(event); + this.closeOtherMenus(); + } + + redirectToFirstChildNavItem(event) { + window.location.href = event.target.getAttribute('data-target-href'); + } + + closeOtherMenus() { + const expandedList = ['expand', 'show']; + const collapsedList = ['collapse', 'hide']; + + this.submenuTargets.map((item) => { + if (item.classList.contains("show")) { + item.classList.remove(...expandedList); + item.classList.add(...collapsedList); + } + }); + } +} diff --git a/app/lib/menu.rb b/app/lib/menu.rb index 6410ce53f..698208326 100644 --- a/app/lib/menu.rb +++ b/app/lib/menu.rb @@ -17,7 +17,7 @@ class Menu @items[name] = { svg: "#{svg}.svg", children: children } end - def item(name:, controller: name, children: []) - { name: name, controller: controller.tr(" ", "_"), children: children } + def item(name:, controller: name, children: [], visible: true) + { name: name, controller: controller.tr(" ", "_"), children: children, visible: visible } end end diff --git a/app/models/admin_menu.rb b/app/models/admin_menu.rb index 2a8c5817b..ba9437ba0 100644 --- a/app/models/admin_menu.rb +++ b/app/models/admin_menu.rb @@ -1,3 +1,5 @@ +# This "model" is not backed by the database. Its main purpose is to +# setup and provide methods to interact with the admin sidebar and tabbed menu class AdminMenu # On second level navigation with more children, we reference the default tabs controller. i.e look at developer_tools # rubocop:disable Metrics/BlockLength @@ -23,6 +25,7 @@ class AdminMenu item(name: "display ads"), item(name: "navigation links"), item(name: "pages"), + item(name: "profile fields", visible: false), ] scope :admin_team, "user-line", [ @@ -34,7 +37,7 @@ class AdminMenu item(name: "mods"), item(name: "moderator actions ads", controller: "moderator_actions"), item(name: "privileged reactions"), - # item(name: "interaction limits", controller: "" ) + # item(name: "interaction limits") ] scope :advanced, "flashlight-line", [ @@ -45,6 +48,7 @@ class AdminMenu item(name: "tools"), item(name: "vault secrets", controller: "secrets"), item(name: "webhooks", controller: "webhook_endpoints"), + item(name: "data update scripts", visible: false), ]), ] @@ -54,11 +58,19 @@ class AdminMenu item(name: "listings"), item(name: "welcome"), ] - end + end.freeze # rubocop:enable Metrics/BlockLength + def self.navigation_items + return ITEMS unless FeatureFlag.enabled?(:profile_admin) || FeatureFlag.enabled?(:data_update_scripts) + + feature_flagged_menu_items + end + def self.nested_menu_items(scope_name, nav_item) - ITEMS.dig(scope_name.to_sym, :children).each do |items| + return unless navigation_items.dig(scope_name.to_sym, :children) + + navigation_items.dig(scope_name.to_sym, :children).each do |items| return items if items[:controller] == nav_item next unless items[:children]&.any? @@ -73,4 +85,25 @@ class AdminMenu scope, nav_item = request.path.split("/").last(2) nested_menu_items(scope, nav_item) end + + def self.feature_flagged_menu_items + # We default to creating a ITEMS constant with visibility set to false + # and then simply amend the visibility of the feature flag when it's + # turned on, instead of creating the payload dynamically each time. + menu_items = ITEMS.dup + + if FeatureFlag.enabled?(:profile_admin) + profile_hash = menu_items.dig(:customization, :children).detect { |item| item[:controller] == "profile_fields" } + profile_hash[:visible] = true + end + + if FeatureFlag.enabled?(:data_update_scripts) + data_update_script_hash = menu_items.dig(:advanced, :children) + .detect { |item| item[:controller] == "tools" }[:children] + .detect { |item| item[:controller] == "data_update_scripts" } + data_update_script_hash[:visible] = true + end + + menu_items + end end diff --git a/app/views/admin/shared/_nested_sidebar.html.erb b/app/views/admin/shared/_nested_sidebar.html.erb index bf9d9a9fe..a1ca11b09 100644 --- a/app/views/admin/shared/_nested_sidebar.html.erb +++ b/app/views/admin/shared/_nested_sidebar.html.erb @@ -1,34 +1,44 @@ <% menu_items.each do |group_name, group| %>
  • <% if group[:children].length == 1 %> - " - href="/admin/<%= group[:children][0][:controller] %>" - aria-page="<%= "page" if deduced_controller(request) == group[:children][0][:controller] %>" - > - <%= inline_svg_tag("#{group[:svg]}", aria: true, class: "dropdown-icon crayons-icon") %> - <%= display_name(group_name) %> - + <% if group[:children][0][:visible] %> + " + href="/admin/<%= group[:children][0][:controller] %>" + aria-page="<%= "page" if deduced_controller(request) == group[:children][0][:controller] %>" + data-action="click->sidebar#expandDropdown" + > + <%= inline_svg_tag(group[:svg], aria: true, class: "dropdown-icon crayons-icon") %> + <%= display_name(group_name) %> + + <% end %> <% else %> <% end %> diff --git a/app/views/admin/shared/_tabbed_navbar.html.erb b/app/views/admin/shared/_tabbed_navbar.html.erb index 7ce3702a3..619b102eb 100644 --- a/app/views/admin/shared/_tabbed_navbar.html.erb +++ b/app/views/admin/shared/_tabbed_navbar.html.erb @@ -1,17 +1,21 @@ -<% nested_menu_items = AdminMenu.nested_menu_items_from_request(request) %> - -<% if nested_menu_items[:children].any? %> -
    -

    <%= nested_menu_items[:name].to_s.titleize %>

    +<% if menu_items.present? && menu_items[:children].any? %> +
    +

    <%= menu_items[:name].to_s.titleize %>

    -
    diff --git a/app/views/layouts/admin.html.erb b/app/views/layouts/admin.html.erb index ab5e88f6a..0cba33346 100644 --- a/app/views/layouts/admin.html.erb +++ b/app/views/layouts/admin.html.erb @@ -51,7 +51,7 @@
    -
    +