Skip to content
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

add descriptions for background pictures #41216

Closed
wants to merge 11 commits into from

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented Oct 31, 2023

Close #40689

I added translations for the attributions and added descriptions for each foto.

@szaimen szaimen added the 2. developing Work in progress label Oct 31, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Oct 31, 2023
github-advanced-security[bot]

This comment was marked as resolved.

Signed-off-by: Simon L <[email protected]>
@szaimen

This comment was marked as resolved.

Signed-off-by: Simon L <[email protected]>
@szaimen szaimen force-pushed the enh/40689/add-foto-description branch from 4727864 to 773fe16 Compare October 31, 2023 15:00
return [
'hannah-maclean-soft-floral.jpg' => [
'attribution' => $this->l10n->t('Soft floral (Hannah MacLean, CC0)'),
'description' => $this->l10n->t('Abstract picture in yellow and white color whith a flower on it'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: Please review each description carefully. Does the text look fine?

Signed-off-by: Simon L <[email protected]>
This reverts commit a6cf2ea.
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Signed-off-by: Simon L <[email protected]>
Signed-off-by: Simon L <[email protected]>
Signed-off-by: Simon L <[email protected]>
Signed-off-by: Simon L <[email protected]>
@@ -57,7 +58,8 @@ public function __construct(Util $util,
ImageManager $imageManager,
IConfig $config,
IL10N $l,
IAppManager $appManager) {
IAppManager $appManager,
BackgroundService $backgroundService) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is now breaking many tests (34 afaics). @ChristophWurst Any other idea how I can do this without having to refactor everything? Adding backgroundservice to this constructor at least does not seem to work...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any other ideas.

Can you adjust/fix the tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I would move the commonthemetrait? Do you think this would make the logic easier?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets see #41235

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could I ask you to try your changes locally? Using CI for trial and error is really expensive and slows down real builds.

@szaimen
Copy link
Contributor Author

szaimen commented Nov 3, 2023

Superceded by #41256

@szaimen szaimen closed this Nov 3, 2023
@szaimen szaimen deleted the enh/40689/add-foto-description branch November 3, 2023 09:05
@szaimen szaimen removed this from the Nextcloud 28 milestone Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BITV]: Replace aria-label's of background buttons with another name
2 participants