Skip to content
This repository has been archived by the owner on May 3, 2021. It is now read-only.

New album doesn't show after create unless you refresh page #135

Closed
jarasSi opened this issue Dec 2, 2018 · 11 comments
Closed

New album doesn't show after create unless you refresh page #135

jarasSi opened this issue Dec 2, 2018 · 11 comments
Labels
bug Something isn't working JS - Lychee-Front JavaScript related see https://github.com/LycheeOrg/Lychee-front
Milestone

Comments

@jarasSi
Copy link

jarasSi commented Dec 2, 2018

Since commit 6ae1838 there is issue when I create new album. Before I refresh my page I cannot see newly created album, and only after that I can enter in it. Up to cfdcf75 after new album had been created it was default working path of my Lychee.

I checked behaviour on multiple browsers - Firefox, Chrome, Vivaldi and Opera. When I revert to
git reset --hard cfdcf752816846376f93c2d47e3fa91fa8036548
problem doesn't exist, but after i pull commit 6ae1838
git reset --hard 6ae18380acd2f4687187714bf81a0727fe50f188
my Lychee acts as described – every time I create new album I have to refresh page just to load new album on the list, enter that album and only than I can upload pictures into it.

@ildyria ildyria added bug Something isn't working JS - Lychee-Front JavaScript related see https://github.com/LycheeOrg/Lychee-front accepted labels Dec 3, 2018
@bennettscience
Copy link
Contributor

This is interesting because using the n hotkey to make a new album, you're immediately taken into that album, so you don't have to refresh. Making an album from the UI exposes the bug because you're not placed into that new album. Should those be made consistent with one another?

@ildyria
Copy link
Member

ildyria commented Dec 12, 2018

In both case you should be placed in the new album. So that is indeed surprising.

@bennettscience
Copy link
Contributor

bennettscience commented Dec 12, 2018 via email

@bennettscience
Copy link
Contributor

bennettscience commented Dec 12, 2018

@ildyria I found the bug, but I can't figure out why a particular change was made. In album.js, line 200:

if (data!==false && isNumber(data)) {
    if(IDs != null) // If used with the hotkey, IDs == null.
    {
	callback(IDs, data, false); // we do not confirm
    }
	else {
	    albums.refresh();
	    lychee.goto(data)
    }
} else {
	lychee.error(null, params, data)
}

What is IDs in there for? When you click 'Create album,' it captures a mouse event, which throws callkback, but that isn't defined, either. Can we just remove the conditional and call albums.refresh()?

I also can't pinpoint where the mouse event is bound. Using the hotkey and clicking the modal 'Create' does not register a mouse event. Using the menu, clicking 'New Album' and then 'Create' does register. I need to dig more there, but this seems to be the culprit.

@ildyria
Copy link
Member

ildyria commented Dec 12, 2018

@d7415
Copy link
Contributor

d7415 commented Dec 12, 2018

Yeah, I had a quick look and saw that, but if you trace it back the callback (as @ildyria pointed to) is used for moving and copying photos. Harder to read, but more elegant than my solution for moving :)

I hadn't spotted that // If used with the hotkey, IDs == null. comment though (maybe because the hotkey wasn't mentioned in the initial issue) so there may at least be a clue to differences there...

@d7415
Copy link
Contributor

d7415 commented Dec 12, 2018

From what I can tell, the menu calls album.add() with no IDs (so null) too, so that might be a dead end :(

@bennettscience
Copy link
Contributor

bennettscience commented Dec 12, 2018

@d7415 I added that comment for context.

I think defining a callback in the button action would solve the problem. I haven't rolled my checkout back to cfdcf75 to check the commit history, though. I need to do more testing to see why the mouseevent is bound from the UI but not from the hotkey on the modal.

@jarasSi
Copy link
Author

jarasSi commented Dec 12, 2018

Actually, I created workaround (just for me, because I don’t know what any of this parameters does) but in main.js
(output from diff -u)

--- old_main.js 2018-12-12 14:46:05.081012192 +0100
+++ main.js     2018-12-12 14:45:29.412525086 +0100
@@ -405,7 +405,7 @@
                api.post('Album::add', params, function (data) {

                        if (data !== false && isNumber(data)) {
-                               if (IDs != null) {
+                               if (IDs != null && callback !=null) {
                                        callback(IDs, data, false); // we do not confirm
                                } else {
                                        albums.refresh();

@jarasSi

This comment has been minimized.

@ildyria
Copy link
Member

ildyria commented Dec 26, 2018

Should be fixed with last commit.

@ildyria ildyria closed this as completed Dec 26, 2018
@ildyria ildyria added this to the v3.2.8 milestone Dec 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working JS - Lychee-Front JavaScript related see https://github.com/LycheeOrg/Lychee-front
Projects
None yet
Development

No branches or pull requests

4 participants