Fix broken "Show whitespace changes" button on MR "Changes" tab
This MR fixes the issue where the Show/Hide whitespace changes button was getting stuck in the Show whitespace changes state once it has been toggled.
The problem was that the w=0
parameter in the URL was getting handled as if it was an integer flag. Since url params are always strings the flag will always result in true
regardless of the value.
This MR fixes the issue by converting the w
flag to an integer when it is read out.
Tests for the component has also been added.
What are the relevant issue numbers?
Closes #52122 (closed)
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Tests added for this feature/bug -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines
Merge request reports
Activity
assigned to @jerasmus
added 1 commit
- 711553b7 - Add changelog entry for broken "Show whitespace changes" button
added 43 commits
-
711553b7...22ab1a82 - 42 commits from branch
master
- ed6d86f6 - Merge branch 'master' into 52122-fix-broken-whitespace-button
-
711553b7...22ab1a82 - 42 commits from branch
mentioned in issue #49526 (moved)
added 1 commit
- 3586b12d - Add unit tests for `compare_version` component
added 1 commit
- 14ae2196 - Update unit tests for `compare_version` component
marked the checklist item Changelog entry added, if necessary as completed
marked the checklist item Tests added for this feature/bug as completed
added 41 commits
-
14ae2196...f5d71ad8 - 40 commits from branch
master
- 3a43243d - Merge branch 'master' into 52122-fix-broken-whitespace-button
-
14ae2196...f5d71ad8 - 40 commits from branch
added 1 commit
- cf3d6407 - Cleanup unit tests for `compare_version` component
@leipert could you please review this MR?
assigned to @leipert
- Resolved by Phil Hughes
- Resolved by Lukas Eipert
@jerasmus Thank you! Looks great, two comments. Thank you for adding the tests!
assigned to @jerasmus
added 59 commits
-
d32cc3d3...2a8c3f6d - 58 commits from branch
master
- 0c3b296a - Merge branch 'master' into 52122-fix-broken-whitespace-button
-
d32cc3d3...2a8c3f6d - 58 commits from branch
added 1 commit
- 2bde04d2 - Move isWhitespaceVisible property into a method
added 1 commit
- 4c48d773 - Add unit tests for comparableDiffs and isWhitespaceVisible
@leipert thanks for the review! I've addressed your comments, please have another look
assigned to @leipert
added 10 commits
-
35c51479...8325a1fc - 9 commits from branch
master
- 91141bb7 - Merge branch 'master' into 52122-fix-broken-whitespace-button
-
35c51479...8325a1fc - 9 commits from branch
@jerasmus LGTM! Moving the thing to a method rather than a
computed
property seems sensible. I don't know what maintainers think though.You should probably run prettier, squash all the commits and set a proper commit message for this smallish change in order to satisfy Danger. In case you need help with that, ping me.
assigned to @jerasmus
added 24 commits
-
4873101c...9791f82b - 9 commits from branch
master
- 64a433b0 - Add changelog entry for broken "Show whitespace changes" button
- f046a420 - Fix `isWhitespaceVisible` method
- e4a1f0f5 - Cleanup `isWhitespaceVisible` method
- 32e188e7 - Cleanup `isWhitespaceVisible` method
- 8afc1c13 - Add merge requests diffs mock data
- 89c07b3b - Add unit tests for `compare_version` component
- 73d1554a - Update unit tests for `compare_version` component
- 6b9245e3 - Cleanup unit tests for `compare_version` component
- 5ef8e530 - Remove unused import
- d204af5c - Change qa class name
- eba9e72d - Move isWhitespaceVisible property into a method
- 8a276365 - Add unit tests for comparableDiffs and isWhitespaceVisible
- 7bb0f73b - Refactor isWhitespaceVisible method
- 0019a376 - Ran prettier on the changed files
- dc47bcbf - Merge branch '52122-fix-broken-whitespace-button' of…
Toggle commit list-
4873101c...9791f82b - 9 commits from branch
added 58 commits
-
dc47bcbf...b868b02c - 57 commits from branch
master
- bc557c80 - Fix broken Show/Hide Whitespace changes button + Add unit tests
-
dc47bcbf...b868b02c - 57 commits from branch
changed milestone to %11.5
added 1 commit
- c23c0f59 - Fix Show/Hide Whitespaces button + Add unit tests
Thanks @leipert, I've addressed your comments, could you please have a look and assign to a maintainer?
LGTM! @iamphill Could you do the maintainer review?
assigned to @iamphill
- Resolved by Phil Hughes
- Resolved by Phil Hughes
assigned to @jerasmus
Thanks @iamphill please have another look.
assigned to @iamphill
mentioned in commit 05af98bf
mentioned in issue gitlab-org/release/tasks#515 (closed)
added devopscreate label