-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Allow to remove the background and select a custom colour + cypress #34696
Conversation
c01c453
to
a35a3e5
Compare
020308e
to
9617ad6
Compare
9617ad6
to
06e2b18
Compare
683db53
to
ebb9b85
Compare
Let's go! Cypress tests are in and green! I'm covering the following use cases:
|
aedceb0
to
7876e7d
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
7876e7d
to
ab82a9a
Compare
Fixed |
ab82a9a
to
37fc45c
Compare
# Needed for some specific code workarounds | ||
TESTING: true | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }} |
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.
For photos, I made the action upload the snapshots folder. It helps to debug sometime. Feel free to add something like:
- name: Upload snapshots
uses: actions/upload-artifact@v3
if: always() // maybe only for failing tests. Does failing() exists ?
with:
name: snapshots
path: cypress/snapshots
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.
They are added on the cypress dashboard automatically
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.
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.
You're right, and I knew that at the time, can't remember why I added that then...
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.
I remember now ! I was not about snapshots, but nextcloud logs:
- name: Extract NC logs
if: always()
run: docker-compose --project-directory cypress logs > nextcloud.log
- name: Upload NC logs
uses: actions/upload-artifact@v3
if: always()
with:
name: nc_logs_${{ matrix.containers }}
path: nextcloud.log
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.
This is a good point!
Great :)
Okay, would be really good to fix this as it will always lead to problems. Shall I create a follow-up issue for this? But honestly it would be much better if that could be fixed directly in this PR...
Yeah, putting it first would be better than the current behaviour, however I think it should be really separate from the grid...
Yes, would be really good to have that. Would you also directly do this in this PR? :) |
It has already been discussed and ruled out :) |
No, this PR is too big, and I'm not doing a rebase danse :) |
bf5eb75
to
a9fe182
Compare
a9fe182
to
d47ca27
Compare
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
d47ca27
to
a869259
Compare
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
Fix #34511
Fix #34449
Fix #34560
Later
background-image-invert-if-bright
background_color
keybackground_image
keyoptimize background image migration job #34677