[full-ci] Small fixes#5985
Conversation
diocas
requested review from
fschade,
kulmann and
pascalwengerter
as code owners
|
@diocas could you run Edit: Also, it seems to have conflicting files. Could you rebase it, too? |
diocas
force-pushed
the
up_small_fixes
branch
from
9521a56 to
c6f0665
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20127/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
c6f0665 to
11c0a2f
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20133/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
5bf56b2 to
11c0a2f
Compare
|
Results for oCISSharingInternalUsers2 https://drone.owncloud.com/owncloud/web/20139/58/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
11c0a2f to
0ae0078
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20175/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
0ae0078 to
6662a72
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20224/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
6662a72 to
6ff3a5b
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20307/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
6ff3a5b to
c62f757
Compare
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20311/38/1 |
diocas
force-pushed
the
up_small_fixes
branch
from
c62f757 to
ae973fe
Compare
|
Rebased with latest master and did some changes:
@kulmann @pascalwengerter can this be merged? Some of it, at least? |
diocas
force-pushed
the
up_small_fixes
branch
from
ae973fe to
1d3f577
Compare
kulmann
left a comment
kulmann
left a comment
There was a problem hiding this comment.
nice, thank you! 🚀 just some small comments, otherwise good to go.
| } | ||
| }) | ||
| .sort((first, second) => { | ||
| // We make the default action based on the order, so put the ones who can be default first |
There was a problem hiding this comment.
The comment is a bit misleading. In fact the default action only picks from actions where canBeDefault evaluates to true. But it totally makes sense to have the (possible) default actions first. Otherwise a right click with multiple available actions might have an action first that is not the default. Nice catch!
There was a problem hiding this comment.
Was this a recent fix? In the past, I had non default apps being used when I clicked a file. But maybe the issue was he fact that every app was having canBeDeFault as true... I'll modify the comment then.
There was a problem hiding this comment.
#5970 should fix that. Could you have a look to confirm?
| return { | ||
| name: app.name, | ||
| img: app.icon, | ||
| icon: app.icon, |
There was a problem hiding this comment.
Oh, does the app provider return icon names from the design system now? 🤔 I thought the icon value from the app provider is always some image url.
There was a problem hiding this comment.
For us, yes, we added the icons in ODS. But this should be unified in the app provider as well (I will check and ask for changes), as I don't think is intuitive to have them mixed.
|
https://drone.owncloud.com/owncloud/web/20581/9/7 A unit test needs to be fixed. |
diocas
force-pushed
the
up_small_fixes
branch
from
1d3f577 to
a36e99b
Compare
|
@phil-davis I've changed the test. The test assumed we can delete the icon, but this is not true if you set it from the extension configuration (and since icon always has a default value...). So I removed this part and ensure that, when setting Also applied @kulmann requests and added a changelog. |
|
@wkloucek https://drone.owncloud.com/owncloud/web/20609/9/2 states |
|
https://drone.owncloud.com/owncloud/web/20609/9/2 @diocas we have made a few fixes today for intermittent annoying things in CI pipelines. Please rebase to latest and force-push and you might get more joy! If it dies again the same, then @wkloucek could look or... |
diocas
force-pushed
the
up_small_fixes
branch
from
a36e99b to
902800a
Compare
|
restarted CI, smoke tests are flaky at the moment, sorry |
|
Results for oCISBasic https://drone.owncloud.com/owncloud/web/20619/51/1 |
|
A smoke-test pipeline failed. I commented in issue #6082 (comment) Edit, update: both smoke test pipelines passed this time! |
|
Results for oCISBasic https://drone.owncloud.com/owncloud/web/20625/51/1 |
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/20625/40/1 |
|
I made PR #6088 on top of this to run full-ci and get the tests passing. If I can achieve that tomorrow morning, then I can push any fixes here to get this passing. |
diocas
force-pushed
the
up_small_fixes
branch
from
902800a to
8362a44
Compare
|
Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/20826/16/1
|
This title appears before the config title is loaded.
Show simple spinner (similar to ocSpiner) if it takes too long to load (> 1s); Show error message when failed to load.
Allow img instead of icon Ensure canBeDefault is honored
If there's an image, it takes precedence over icon (which always has a default)
diocas
force-pushed
the
up_small_fixes
branch
from
8362a44 to
6c738c7
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
I'll merge this into a temporary PR to be able to work directly on it |
5 participants
Just a bunch of (I hope) fixes:
Clean paths for drawio(opened a diff PR with this fix)Some more debatable fixes:
I've changed the default title of the html page from "owncloud" to "loading" (not translated). Ofc this changes to the configured title once the app loads, but if something fails or takes a long time to load, users get confused.(edit: made the title configurable instead)In the same spirit, I've added information that is displayed before the app loads:
a loading messagea spinner that appears if it takes more than 1s) to load resources or an error message if something fails. When we had our performance issues, users were getting blank pages and had no idea what was happening, so a bit of feedback is positive. This is not translated, but the users only see the text on error, so it should be acceptable like this.Fixes #5961: it doesn't improve the performance, but a least gives feedback to the user instead of a white page