Compatibility for Nextcloud 28 #1116
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The highlight of the active view was not visible on NC28. Also, we now highlight also the hovered menu item with a background color like the native apps of Nextcloud do in the recent versions. TODO: The rounded background highlight is now visible also on the old NC versions which use square layout everywhere. This doesn't look very nice.
…w on NC28 Previously, the externally mounted folders have had automatically their parent folder set as users home folder (by the cloud core). In NC28, they seem to have parent folder set as -1. Handle this special case separately.
This no longer came from the cloud core by default.
- Fix navigation pane item highlight being shown as rounded on NC17-24 (at least) where the overall layout is not a good fit for this. There still is such an issue on the appearance on NC17 that the active navigation pane item is highlighted with a colored background while this is not the normal graphical style on that NC version. This isn't critical in any way and very few people should use such an old NC version these days. - Don't apply bottom margin for navigation pane items on the older NC versions (< 25) or OC as that's not the norm on those cloud versions - Fix Settings view icon being hidden on the navigation pane on NC13-17 (at least).
This is how it used to be on NC25-27 and now also on NC28.
Only plays individual files and support fo any other cloud version is broken at the moment.
TODO: This doesn't currently work for the publicly shared folders. Also, we don't currently show the loading animation on NC28 during the playlist parsing.
The first prototype of NC28-compatibility intentionally broke the compatibility with any other cloud, but now this is fixed. Disclaimer: so far, the only older cloud tested is NC27.
The old API for registration is no longer available on NC28 and a new one has to be used. The core functionality of the view is now implemented in our stand-alone class, not inheriting any core interface. It is then just wrapped differently depending on the cloud version used. What doesn't currently work on NC28, is starting playback of a new list by clicking a song name in the playlist tab view. This still works on the older clouds, though.
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
01ad7c3
to
8761ea7
Compare
The only partially initialized tab view tried to react on the change of currently playing file.
The highlight of the menu didn't look very nice on the recent NC versions, probably starting from NC25. Moving the padding to an inner element fixed this without harming the layout on OC. The padding is needed in the first place for OC, NC applies enough padding here natively. Also, the "more" button opening the menu was misplaced a bit on the recent NC versions.
The new global search modal used on NC28 was not really suitable to use as an input field for the local search within the active Music app view. And even the "unified search" introduced a few NC major version ago was a bit awkward to use as input for our search. We now have our own search input field docked to the bottom of the navigation pane, just above the Settings link.
- The collapsed navigation pane is expanded when search is started with ctrl+F - The navigation pane is collapsed again, when the search input is cleared or when enter key is hit in the search input field - It's important to take care that the pane collapsing is not triggered twice from the same input since this somehow messes up the collapse/expand button and it no longer reacts to all clicks. As a case in point, the collapseNavigationPaneOnMobile() must not be called when the "clear" button of the search input is clicked since the cloud core already collapses the pane in that case.
… NC25+ Using the variable --body-container-margin to adjust the position of these items didn't work correctly because this variable gets value 0 on the mobile layout. Meanwhile, the cloud core uses the expression calc(var(--default-grid-baseline)*2) for similar purpose on normal navigation pane items, and this remains the same regardless of the window width.
On ownCloud (and on very old versions of Nextcloud) the navigation pane is just 250px wide instead of 300px used on Nextcloud for many years now.
The button was being shown off the vertical middle of the text box on Chrome while it looked nice on Firefox. Now it looks the same on both of these browsers. Also, fine-tuned the horizontal placement of the clear search button and the vertical placement of both the search and playlist name inputs.
…NC28 Because of the limitations of the new API, this logic doesn't work exactly the same way as on the older cloud versions: - In the older clouds, the play order of tracks matches the order selected on the files app (by name/size/date; ascending or descending) - On NC28, the files are always played in ascending order by the file name - What's even more unfortunate is that the play order cannot match the order of any of the non-directory views like Favorites, Recent, or Shares. So even if the playback starts from one of these views, the play queue is still based on the parent directory of the file in question.
… NC28 The OCA.Files.Sidebar API is now used to open the sidebar instead of mFileList.showDetailsView whenever available. On NC28, the Sidebar API is the only way to do this but it has actually been available for more than 4 years, starting from NC18. The benefit of the new API is that it works also in case the target file resides outside the currently viewed folder.
…ixes - It turned out that appending the OC.requestToken to the play URL was not totally unnecessary after all: without it, using the Aurora.js fallback player (to play e.g. AIFF files) didn't work although all the natively supported files played fine. - The logic to show "Import playlist" or "Import radio" didn't work correctly because it was based on the assumption that only external streams may have the property `url` set
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
b434501
to
04cb463
Compare
The method in question doesn't exist before NC28 but we already check for that so there's no point to nag about it.
In the previous NC28-compatibility changes, we made the backend include the
playback URLs when it parses a playlist file and returns its contents in the
REST method `/api/playlists/file/{fileId}`. This had such a drawback that it
increased the parsing time ~25% and the response size by ~50% which could make
a difference with huge playlist files. Meanwhile, we don't actually need the
playback URL for any other song except the one which is just being played.
This was now changed so that the playback URL is created on the front-end
based on the file ID whenever the playing song changes. The logic to create
the playback URL is the same as is used in the Music app proper.
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
194b78c
to
87535d3
Compare
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
87535d3
to
c4f4049
Compare
Unlike previously assumed, the playlist tab must pass also the `path` property of the playlist file for proper functionality in all cases. Without it, the problematic case was the following: 1. Without opening the playlist file, open its details pane and navigate to the Playlist tab 2. Click a song name and the playlist starts playing 3. From the embedded playlist bar, open the menu and select "Show playlist" => empty sidebar was shown
To mimic how this works on the Files app of the recent Nextcloud versions.
NC28 has changed again the border style of the input fields and the CSS variables used for the border colors. On the other hand, the styles we assigned for the special "chosen" input fields never quite matched the styling of the standard input fields on OC or old NC versions. To get consistent styling of all filter inputs on all possible host clouds, we now copy the border style rules from standard input fields to the chosen fields. This required quite a bit of CSS/JS-trickery because we can't directly get or set the :hover style of an element.
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
1c4ae14
to
8d7e552
Compare
Previously, we used to show the "in progress" spinner on the file thumbnail in the files list. This happened when loading or importing a playlist file. NC28 no longer provides an API to show such progress animation and we have changed the design: The animation is now shown on the player bar, overlaid on the album art. This design works the same regardless of the cloud version.
While the externally mounted folders have their parent ID set as -1 on NC28, the folders shared with the current user have the parent ID of the parent folder in the share owner's file system. Both of these cases are now handled on the front-end and the back-end-side fix previously added for the externally mounted folders was ditched.
Importing the '@nextcloud/files' broke the compatibility with Internet Explorer. This was solved by importing this module dynamically, only when actually used.
paulijar
changed the title
Our previous way of using `method_exists` for `OCP\Util\addInitScript` was flagged by PHPStan. Writing the method call a bit differently solved this. Unfortunately, Scrutinizer is a bit more stupid here and still requires an inline suppression also with this modified version.
paulijar
force-pushed
the
feature/NC28-compatibility
branch
from
a37e2f1
to
96f7956
Compare
|
The inspection completed: No new issues |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
To be fully compatible with Nextcloud 28, quite a few issues need to be fixed.
In the Music app proper:
In the embedded player within the Files app: