-
Notifications
You must be signed in to change notification settings - Fork 8.5k
DEV: Refactor tests & add animation #32547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
spec/system/header_search_spec.rb
Outdated
|
||
RSpec.describe "Header Search - Responsive Behavior", type: :system do | ||
fab!(:user) | ||
fab!(:topics) { Fabricate.times(20, :post).map(&:topic) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to do this, it's slow and we have a "cheat" way of doing this type of thing where you need to scroll a lot vertically:
discourse/spec/support/system_helpers.rb
Lines 178 to 187 in abee294
def fake_scroll_down_long(selector_to_make_tall = "#main-outlet") | |
# Trick to give a huge vertical space to scroll | |
page.execute_script( | |
"document.querySelector('#{selector_to_make_tall}').style.height = '10000px'", | |
) | |
sleep 0.1 # most resilient solution for now | |
page.scroll_to(0, 1000) | |
end |
fake_scroll_down_long |
spec/system/header_search_spec.rb
Outdated
visit "/" | ||
|
||
# Default desktop view should show search field | ||
expect(page).to have_no_css(".floating-search-input") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a page object for search related system spec stuff already https://github.com/discourse/discourse/blob/abee294616a064e4de40d8e8eb052ea571e2203c/spec/system/page_objects/pages/search.rb , you should use this where possible in this spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with caveat that the system spec needs a bit of extra work
@martin-brennan would you mind going through again? I added in all of your reccomendations |
This PR:
New
new-scroll-headersearch.mp4
Old
old-scroll-headersearch.mp4