Fix scroll jump voting investments #3113
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
spec/features/budgets/ballots_spec.rb:112
("Ballots Voting Adding and Removing Investments Add a investment"),spec/features/budgets/ballots_spec.rb:282
("Ballots Groups Change my heading") andspec/features/budgets/ballots_spec.rb:615
("Ballots Permissions Insufficient funds (removed after destroying from sidebar)")Explain why the test is flaky
When users scroll the page, the header nav bar changes its position on the screen to a fixed one, causing the space it was in to suddenly become empty, making the page scroll even further. If the user is a Capybara driver, that extra scroll might end on it not clicking the link it was trying to click.
Explain why your PR fixes it
Given the complexity of the elements surrounding our header nav bar and the CSS rules applied to it and its changes in height and font size, the easiest way to fix the problem is to remove the padding of this element, which is exactly the space which will be left empty when the element changes its position to a fixed one.
I've tried more complex solutions like adding extra elements with a padding that compensates the empty space or manually adding extra padding, as suggested in the articles above. However, maybe because the header nav bar changes in height and font size, these solutions looked worse, with the scroll never being smooth enough and the header getting more padding than it has now.
On the other hand, the chosen solution might be fragile and we might have this issue again if we change the related CSS.
Notes
There are other ways to make the affected tests pass without changing the code, like changing CSS rules for the test environment or increasing the window size of the Capybara driver so it doesn't need to scroll. However, IMHO these tests are failing because we've got a bug in our code (in this case, affecting usability). Changing the test so it passes without fixing the bug would make our test suite worse than it is now, since it wouldn't detect this usability issue.