Resolve "When the branch name is a divider or separator, the project home page cannot display the branch name correctly."
What does this MR do?
This MR fixes an issue where branches named separator
or divider
don't show up in the project ref switcher, but do cause it to render an actual divider or separator dropdown item.
Problem
The gl_dropdown.js
file reads in string primitives for each data item. If these strings match either divider
or separator
, the file doesn't render the data, and instead renders an actual divider or separator item.
Unfortunately, "separator"
and "divider"
are much too easy to accidentally conflict with.
Solution
This MR completely refactors the rendering portion(s) of the gl_dropdown.js
file.
All divider
s and separator
s must be generated by { "type": ("separator"|"divider") }
instead of plaintext strings.
More solution notes from commit c91f915a:
Basic Intent: Allow all branch names without accidentally creating layout or backstage DOM. e.g. a branch named
separator
should never create a separatorli
element.Ideally, there should never be a string that could cause this kind of conflict.
Implementation: All of
GitLabDropdown.renderItem
is extracted to a standalone module.To render a divider or separator, consumers must now pass in an object like
{ "type": "divider" }
or{ "type": "separator" }
Notable choices:
- All of the functions have a cyclomatic complexity of 3 or less
- See: https://en.wikipedia.org/wiki/Cyclomatic_complexity
- Note the "Correlation to number of defects" section
- While software complexity may not have a directly causal relationship with defects, less complex software is generally easier to reason about, and may reduce defects. I personally try to maintain complexity of no higher than 3.
Other Notes
There's an epic issue that addresses the double scrollbar problem that occurs when the branch/tag/commit list is scrollable, which is visible in the after
screenshot (since the list is now long enough to scroll, barely), or on gitlab.com
.
Screenshots
Before | After |
---|---|
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry for user-facing changes, or community contribution. Check the link for other scenarios. -
Documentation created/updated or follow-up review issue created -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Performance and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers
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
Closes #65473 (moved)