[Enhancement] Add a checkbox to overwrite or add tags when selecting mulitple images

[trigun539]

Steps to reproduce:

• Tag some pictures and add multiple tags to them.

• Now, select multiple pictures and click on tag selected:

• Click on the initial picture and you'll see the old tags gone.

As you can see, the old tags have all been replaced when you add new tags. It should've kept the old tags, otherwise, it'll be really hard to be able to tag pictures.

Thanks

[d7415]

Ok, well firstly it's not as clear cut as you say - in that case how would you remove a tag from all of those photos?

Secondly, the dialog clearly says "Existing tags will be overwritten", so this is in no way unexpected behaviour.

Unless someone proposes (and, if we're realistic, implements) a UI that allows for the proposed and current behaviours without adding unnecessary complexity, I don't see this changing.

I'm going to close this unless/until that happens.

[trigun539]

@d7415 To me this seems like a big usability issue. This makes tagging in the application unusable since users will tag some pictures here and there, and then they'll add new tags which would remove any previous tags (message saying that it'll remove the old tags doesn't really help). Instead of replacing the tags, it should just add new tags.

For clearing tags, maybe having a small tag list and user can remove the tag completely? Or adding a dropdown option to clear tags for the selected pictures?

[d7415]

Instead of replacing the tags, it should just add new tags.

You're welcome to this opinion. I strongly disagree, and I did not build that UI, so from a very small sample you are in the minority.

As I said, I have no objections to this functionality but it shouldn't be at the expense of simplicity or of users expecting the current behaviour.

For clearing tags, maybe having a small tag list and user can remove the tag completely?

Which tags? Where is the list? Completely from the selected pictures or from the whole gallery?

Or adding a dropdown option to clear tags for the selected pictures?

This adds an extra step for users expecting the existing behaviour, as they would have to clear the tags and then re-add them. We're also slightly reluctant to add extras options here unless there is a clear benefit. A "clear tags" button with the list of tags could work, but wouldn't help with multiple photos.

It sounds like you have lots of tags that each affect many photos, but with very few photos sharing the same set of tags, which may be a use case we're not fully catering for at the moment. Again, my issue is that the initial proposal is replacing existing (desired) functionality with your preferred version.

I guess the simplest option might be a configuration toggle. If that sounds like a sensible compromise let us know. I'll leave reopening to someone else as I don't have the time right now to consider the pros/cons of such an approach.

[ikosa]

+1 for @trigun539 i think the existing behavior is useless a checkbox to overwrite or add tags will be great

[trigun539]

@ildyria My PHP is pretty rusty, but I could take a look at this feature if someone can point me in the right direction (which files would need to be changed).

[ildyria]

Some readings:

Introduction to the MVC of lychee: https://lycheeorg.github.io/docs/read-more.html
Directory structure of the backend: https://lycheeorg.github.io/docs/structure.html

Files you will need to modify:
https://github.com/LycheeOrg/Lychee/blob/master/routes/web.php
https://github.com/LycheeOrg/Lychee/blob/master/app/Http/Controllers/PhotoController.php#L299

If you need to make a more complex function, I highly advise you to create a new function in:
https://github.com/LycheeOrg/Lychee/blob/master/app/ModelFunctions/PhotoFunctions.php

To modify the front end, have a look at this: https://lycheeorg.github.io/docs/node.html
More precisely, you will most likely need to modify (and then recompile):
https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/photo.js#L847
https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/lychee.js#L158

To add settings to the database, you will need to create a migration file:

php artisan make:migration <name of your migration>

which will then be executed with:

php artisan migrate --step

(--step is optional but it allows to perform migration iteratively instead of all in one go) or rolled back

php artisan migrate --rollback

The create file will be added in this folder:
https://github.com/LycheeOrg/Lychee/tree/master/database/migrations
Ideally you want to add a default setting, here is an example of how to do it:
https://github.com/LycheeOrg/Lychee/blob/master/database/migrations/2020_06_04_104605_config_editor_enabled.php
Look at the other migration files to get a broader idea of how this works. It is not that complex.

As we have CI to unify our code style, before creating a pull request, you will need to check that you are properly formatting the code. You just need to execute the following command and it will take care of fixing the style:

make formatting

Lots of information, but once you got a rough idea of how the full things works, it is not that complex (contrary to what @d7415 thinks 😝 )

[trigun539]

@ildyria just tried to run vagrant up, but seems like I need some homestead.yaml / homestead.json config file. Do you have one that I could use?

[ildyria]

I don't use vagrant, so I can't really help you with that one. However I do know that Laravel support Homestead: https://laravel.com/docs/7.x/homestead and I think our repo comes with it prepackaged (indeed: https://github.com/LycheeOrg/Lychee/blob/master/composer.json#L39) if you use the composer install without the --no-dev option. So there should be something in the vendor folder related to it.

[ildyria]

Additionally there is this : https://github.com/LycheeOrg/Lychee/blob/master/Vagrantfile

[trigun539]

@ildyria getting this error when running Lychee. Do you happen to know what I'm missing?

The installation seemed to finish successfully.

[ildyria]

are you using vagrant or Nginx because I can't really help you in that case as I am an apache user. :(

Did you had a look here: https://lycheeorg.github.io/docs/faq.html#when-i-do-x-i-get-an-error-api-not-found-what-can-i-do ?

[trigun539]

Yes, doing vagrant / nginx / homestead since seemed to be the easier one to setup. I'll take a look on the link you sent.

[ildyria]

@d7415 😃

[trigun539]

Updated nginx conf file with the settings given on link and seems to be working 👍

[anderaith]

I answer to this issue only to say that I strongly agree that it is needed.
I have a big library of images used as references and with the current behavior, I can't tag them and search them properly.
I'm using Piwigo at the moment because of that and it's definitely not as user friendly as Lychee.

[ildyria]

Yes but sadly, we are not many dev on the project, so it is hard to allocate time and resources to it. There are so many things that need to be developed... E.G. the ability to specify how you sort an album PER album instead of a global setting.

[trigun539]

@ildyria Got the backend sorted out for this, but frontend still needs some work. Can you send me some idea how the frontend is put together. Seems like it's using leaflet for everything? Is lychee a custom frontend MVC? I see lychee.html in a bunch of places which seems to be the way to display HTML, other than base Laravel views.

[ildyria]

The front end is to be compiled from here : https://github.com/LycheeOrg/Lychee-front
https://github.com/LycheeOrg/Lychee-front/blob/d2bd2a32b9d9db4516441f08ae94b125856e79d9/scripts/main/photo.js#L849

lychee.html is a function call which do replace search and replace:
https://github.com/LycheeOrg/Lychee-front/blob/d2bd2a32b9d9db4516441f08ae94b125856e79d9/scripts/main/lychee.js#L780

Also worth reading: https://lycheeorg.github.io/docs/node.html

[trigun539]

@ildyria I'll take another look

[skyghis]

I'm interested in the "add tag" feature.
Is the development of the feature still in progress ?

[moritzhoewer]

I recently started using Lychee and also realized I need this badly. Lacking time to implement it properly, I "hacked" the setTags function of the PhotoController (https://github.com/LycheeOrg/Lychee/blob/master/app/Http/Controllers/PhotoController.php#L219) and replaced it with

	public function setTags(SetPhotosTagsRequest $request): void
	{
		$tags = $request->tags();

		// Hack: If editing multiple photos append, if editing one photo replace	
		$append = count($request->photos()) > 1;

		/** @var Photo $photo */
		foreach ($request->photos() as $photo) {
			if($append) {
				$photo->tags = array_unique(array_merge($photo->tags, $tags));
			} else {
				$photo->tags = $tags;
			}
			$photo->save();
		}
	}

So basically, if I am editing a single photo it replaces the tags, as before. But if I am editing multiple photos, then it will append the tags (removing potential duplicates).

[qwerty287]

@moritzhoewer you can open a pull request, this hack looks pretty good already. You should add some request parameter that allows switching between override / append but otherwise, this looks nice. I can also check it later and open a pull request.

[qwerty287]

I just opened #1564 which adds support for this, based on @moritzhoewer's hack.

[moritzhoewer]

Nice, thank you for turning this into a proper feature - with frontend, tests, localization and everything. As I said in the original message, I unfortunately didn't have time to implement more than this hack...it's been a while since I last worked with PHP and I never used Laravel. I could probably have managed with enough time, but it would have taken much longer than it took you 😁