[full-ci] Fix search bar display on small screens #7675
Conversation
|
Results for oCISFiles2 https://drone.owncloud.com/owncloud/web/28686/57/1 💥 The acceptance tests failed on retry. Please find the screenshots inside ...
webUIFilesSearch-search_feature-L121.pngwebUIFilesSearch-search_feature-L141.pngwebUIFilesSearch-search_feature-L78.pngwebUIFilesSearch-search_feature-L95.png |
JammingBen
changed the title
JammingBen
force-pushed
the
searchbar-on-small-screens
branch
from
604b60f to
13e9c66
Compare
| }, | ||
| switchText() { | ||
| return this.isLightTheme ? this.$gettext('Light mode') : this.$gettext('Dark mode') |
There was a problem hiding this comment.
Unrelated but fixed in the course of this: removed the label of the currently used theme as the icon + tooltip is indicator enough.
JammingBen
force-pushed
the
searchbar-on-small-screens
branch
from
499921f to
953141d
Compare
|
@individual-it One acceptance test keeps failing here: https://drone.owncloud.com/owncloud/web/28569/57/14. However, on my local machine, the test succeeds. I have no idea what's the issue here... Maybe your team could have a look if time permits? |
@JammingBen, I was working separately to solve acceptance failure in #7692 PR. But mistakenly I pushed my commit to this PR in place of #7692 PR. Sorry for inconvience. You can drop my commit. From my investigation, for
I found something odd in the search feature. In the tests run, I noticed that after renaming resources or uploading resources Personal page contained renamed/uploaded resources. But if we search currently renamed/uploaded, then renamed/uploaded resources can't be found. This issue is reproducible only in automation testing. |
JammingBen
force-pushed
the
searchbar-on-small-screens
branch
from
e08a813 to
f3104ba
Compare
That's strange though, as this PR only touches visuals. Also, the failing test suite is for iPhone only (meaning on small screens). Are you sure that the search bar is being accessed the right way during the test? This PR changed the behavior on mobile devices so the search icon has to be clicked first, then the search input appears. Maybe the time between the click and the appearing search input is too small and we need to wait for the search input to be fully visible and usable? |
|
@JammingBen the indexing of uploaded files for search done async could be the culprit behind |
|
I believe that it worked before because the item |
JammingBen
force-pushed
the
searchbar-on-small-screens
branch
from
f3104ba to
ea90072
Compare
|
@individual-it I added a pause statement as discussed in the daily yesterday: f77d040 Is that okay from your side (at least for now)? |
|
@JammingBen yes lets do it that way. I cannot think of any other way to do it :-( |
| }, | ||
| onResize() { | ||
| const searchBarEl = document.getElementById('files-global-search-bar') | ||
| if (window.innerWidth > 639) { |
There was a problem hiding this comment.
shouldnt this be possible via css? I mean its fine with me just weird to see this number in code rather than css
There was a problem hiding this comment.
We need it unfortunately to set the visible state back again if you search on a small screen and resize your screen to full again. Without, the search would stay hidden because showSearchBar() actively sets style.visibility = 'visible'.
|
SonarCloud Quality Gate failed. |
Description
We've fixed the display of the search bar on small screens.
We also removed the label of the currently used theme as the icon + tooltip is indicator enough.
Related Issue
Screenshots:
See issue.
Types of changes