Skip to content
Snippets Groups Projects

Resolve "Add and edit custom Wiki sidebar support from the UI"

6 unresolved threads

What does this MR do?

Implements #23109 (closed)

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Markus Koller

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
8 8 = link_to git_access_url, class: active_nav_link?(path: 'wikis#git_access') ? 'active' : '', data: { qa_selector: 'clone_repository_link' } do
9 9 = icon('cloud-download', class: 'append-right-5')
10 10 %span= _("Clone repository")
11
11 = link_to git_access_url, class: active_nav_link?(path: 'wikis#_sidebar#edit') ? 'active' : '', data: { qa_selector: 'edit_sidebar_link' } do
12 = sprite_icon('cloud-download', size: 16, css_class: 'gl-mr-2')
13 %span= _("Edit Sidebar")
  • @lifrank1994 there's a few things we need to change here:

    • git_access_url is not the correct path yet, we can use wiki_page_path(@wiki, Wiki::SIDEBAR, action: :edit).
    • active_nav_link? doesn't work here as it can only detect the current controller and action, but not which wiki page we're editing. We also can't rely on the action as the edit template can also be rendered by the show and update actions. I think the easiest way around this would be to just pass the current view name from edit.html.haml.
    • The cloud-download icon doesn't exist in our SVG sprites, you can see all available icons at https://gitlab-org.gitlab.io/gitlab-svgs and I think here pencil or pencil-square would make sense. (for context: the icon helper is legacy code and uses Font Awesome, if you rebase master you'll see that we recently swapped the existing icon call above to use sprite_icon as well).

    So putting it all together, I came up with this which seems to work as expected:

    diff --git i/app/views/shared/wikis/_sidebar.html.haml w/app/views/shared/wikis/_sidebar.html.haml
    index cddf19fbc8e..9fb4261152d 100644
    --- i/app/views/shared/wikis/_sidebar.html.haml
    +++ w/app/views/shared/wikis/_sidebar.html.haml
    @@ -1,3 +1,5 @@
    +- current_view ||= nil
    +
     %aside.right-sidebar.right-sidebar-expanded.wiki-sidebar.js-wiki-sidebar.js-right-sidebar{ data: { "offset-top" => "50", "spy" => "affix" } }
       .sidebar-container
         .block.wiki-sidebar-header.gl-mb-3.w-100
    @@ -9,6 +11,12 @@
             = sprite_icon('download', size: 16, css_class: 'gl-mr-2')
             %span= _("Clone repository")
     
    +      - edit_sidebar_path = wiki_page_path(@wiki, Wiki::SIDEBAR, action: :edit)
    +      - edit_sidebar_class = (current_view == 'edit' && @page&.slug == Wiki::SIDEBAR) ? 'active' : ''
    +      = link_to edit_sidebar_path, class: edit_sidebar_class, data: { qa_selector: 'edit_sidebar_link' } do
    +        = sprite_icon('pencil-square', size: 16, css_class: 'gl-mr-2')
    +        %span= _("Edit Sidebar")
    +
         .blocks-container
           .block.block-first.w-100
             - if @sidebar_page
    diff --git i/app/views/shared/wikis/edit.html.haml w/app/views/shared/wikis/edit.html.haml
    index 64a4816def6..4149b6b821c 100644
    --- i/app/views/shared/wikis/edit.html.haml
    +++ w/app/views/shared/wikis/edit.html.haml
    @@ -24,4 +24,4 @@
     
     = render 'shared/wikis/form', uploads_path: wiki_attachment_upload_url
     
    -= render 'shared/wikis/sidebar'
    += render 'shared/wikis/sidebar', current_view: 'edit'
  • @lifrank1994 oh and we also need to wrap the whole thing in a permission check :see_no_evil: So the link should only be visible if the user actually has permission to edit.

    The code for that is can?(current_user, :create_wiki, @wiki.container), but that's a lot of logic in the view now so we should probably extract it into a helper method.

  • /home/frank/gitlab-development-kit/frankfork/gitlab/app/helpers/wiki_helper.rb

  • return unless error

  • @toupeira what does this line do with the view? = render 'shared/wikis/sidebar', current_view: 'edit'

  • what does this line do with the view? = render 'shared/wikis/sidebar', current_view: 'edit'

    @lifrank1994 this defines a local variable current_view in the shared/wikis/sidebar template. We also need to initialize the variable with - current_view ||= nil, to make sure we don't get an exception on other views.

  • changed this line in version 3 of the diff

  • Please register or sign in to reply
  • 8 8 = link_to git_access_url, class: active_nav_link?(path: 'wikis#git_access') ? 'active' : '', data: { qa_selector: 'clone_repository_link' } do
    9 9 = icon('cloud-download', class: 'append-right-5')
    10 10 %span= _("Clone repository")
    • @tomquirk the icons are currently not well aligned, I'm wondering if there's an easy fix for that:

      image

      The layout also breaks when you shrink the window:

      image

      Maybe we should just have each link on their own line for now?

      image

      (not sure how to actually do that properly, I just quickly tested by wrapping both in a <div>)

      /cc @mnearents

    • @toupeira here is what I came up with!

      Feel free to ping me for frontend review if you need it eventually (and nice work so far @lifrank1994 :bow: )

      Full-width responsive
      Screen_Shot_2020-07-24_at_11.33.32_pm Screen_Shot_2020-07-24_at_11.33.23_pm
      diff --git a/app/views/shared/wikis/_sidebar.html.haml b/app/views/shared/wikis/_sidebar.html.haml
      index 24b4ad98ddd..5d2251614f6 100644
      --- a/app/views/shared/wikis/_sidebar.html.haml
      +++ b/app/views/shared/wikis/_sidebar.html.haml
      @@ -3,14 +3,15 @@
           .block.wiki-sidebar-header.gl-mb-3.w-100
             %a.gutter-toggle.float-right.d-block.d-sm-block.d-md-none.js-sidebar-wiki-toggle{ href: "#" }
               = sprite_icon('chevron-double-lg-right', size: 16, css_class: 'gl-icon')
      -
      -      - git_access_url = wiki_path(@wiki, action: :git_access)
      -      = link_to git_access_url, class: active_nav_link?(path: 'wikis#git_access') ? 'active' : '', data: { qa_selector: 'clone_repository_link' } do
      -        = icon('cloud-download', class: 'append-right-5')
      -        %span= _("Clone repository")
      -      = link_to git_access_url, class: active_nav_link?(path: 'wikis#_sidebar#edit') ? 'active' : '', data: { qa_selector: 'edit_sidebar_link' } do
      -        = sprite_icon('cloud-download', size: 16, css_class: 'gl-mr-2')
      -        %span= _("Edit Sidebar")
      +      
      +      .gl-display-flex.gl-flex-wrap
      +        - git_access_url = wiki_path(@wiki, action: :git_access)
      +        = link_to git_access_url, class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap gl-mr-5' + (active_nav_link?(path: 'wikis#git_access') ? ' active' : ''), data: { qa_selector: 'clone_repository_link' } do
      +          = sprite_icon('download', size: 16, css_class: 'gl-mr-2')
      +          %span= _("Clone repository")
      +        = link_to git_access_url, class: 'gl-display-flex gl-align-items-center gl-white-space-nowrap' + (active_nav_link?(path: 'wikis#_sidebar#edit') ? ' active' : ''), data: { qa_selector: 'edit_sidebar_link' } do
      +          = sprite_icon('pencil-square', size: 16, css_class: 'gl-mr-2')
      +          %span= _("Edit sidebar")
           .blocks-container
             .block.block-first.w-100
               - if @sidebar_page
      
    • Please register or sign in to reply
  • Markus Koller changed the description

    changed the description

  • To build a test gitlab/spec/helpers/wiki_helper_spec.rb

    Want to create context of user. Look around for something like this

    allow(helper).to receive(:can?).with(user, :read_cross_project).and_return(true)

    Edited by Frank L
  • look into gitg

  • Frank L added 1 commit

    added 1 commit

    • 4d62d3f5 - Apply 1 suggestion(s) to 1 file(s)

    Compare with previous version

  • Frank L added 2 commits

    added 2 commits

    • cc962bda - code review changes
    • c7209d54 - sidebar working although need to fix formatting

    Compare with previous version

  • added backend label

  • @lifrank1994 hey Frank, I wanted to check in what your status is on this :grinning:

    There's still a few open discussions that need to be addressed, let me know if I can help or anything is unclear!

  • mentioned in issue #17347 (closed)

  • reopened

  • 🤖 GitLab Bot 🤖 added groupeditor [DEPRECATED] label and removed 1 deleted label

    added groupeditor [DEPRECATED] label and removed 1 deleted label

  • Closing in favour of !50323 (merged)

  • closed

  • Markus Koller mentioned in merge request !50323 (merged)

    mentioned in merge request !50323 (merged)

  • Please register or sign in to reply
    Loading