Skip to content

GitLab Next

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
GitLab FOSS
GitLab FOSS
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 0
    • Issues 0
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
    • Iterations
  • Merge requests 0
    • Merge requests 0
  • Requirements
    • Requirements
    • List
  • Operations
    • Operations
    • Incidents
    • Environments
  • Packages & Registries
    • Packages & Registries
    • Container Registry
  • Analytics
    • Analytics
    • Code Review
    • Insights
    • Issue
    • Repository
    • Value Stream
  • Snippets
    • Snippets
  • Members
    • Members
  • Activity
  • Graph
  • Create a new issue
  • Commits
  • Issue Boards
Collapse sidebar
  • GitLab.org
  • GitLab FOSSGitLab FOSS
  • Merge requests
  • !8773

Merged
Created Jan 25, 2017 by Eric Eastwood@MadLittleModsContributor7 of 7 tasks completed7/7 tasks

Fix permalink discussion note being collapsed

  • Overview 9
  • Commits 1
  • Pipelines 6
  • Changes 3

What does this MR do?

Permalinked MR discussion comments will no longer collapse.


What causes the bug?

The notes tab panes(.notes.tab-pane) is hidden initially until it gets a .active class after the JS kicks in merge_request_tabs.js.es6 -> Contstructor -> activateTab

Previously, if there was a hash in the URL, #notes_xxx, we were clicking the toggle button if the area was hidden, toggler_behavior.js; which even though the discussion is expanded in the server-side render, is actually considered hidden because the hidden notes tab parent,

Summary: If the notes tab doesn't become visible in time, then we click the toggle button which hides the linked discussion. race-condition

You can see the behaviors(toggler_behavior) get required higher up in the bundle than the merge_request/merge_request_tabs: https://gitlab.com/gitlab-org/gitlab-ce/blob/1138afe7c0c0ae2b80b5282aff42e6399328eea8/app/assets/javascripts/application.js#L54-62

toggler_behaviour runs inside a jQuery ready while merge_request is just in an IIFE

Are there points in the code the reviewer needs to double check?

Why was this MR needed?

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

  • Changelog entry added
  • Tests
    • Added for this feature/bug
    • All builds are passing
  • Conform by the merge request performance guides
  • Conform by the style guides
  • Branch has no merge conflicts with master (if it does - rebase it please)
  • Squashed related commits together

What are the relevant issue numbers?

Closes #26860 (closed)

Closes #27089 (closed)

Closes #27151 (closed)

Assignee
Assign to
Reviewer
Request review from
8.16
Milestone
8.16 (Past due)
Assign milestone
Time tracking
Source branch: 27089-26860-27151-fix-discussion-note-permalink-collapsed