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

[mod] reverse log order #250

Open
wants to merge 4 commits into
base: stretch-unstable
from

Conversation

@Psycojoker
Copy link
Member

commented Jun 2, 2019

I kept being bothered that the log where in reverse order so I end up trying to find a way to change that.

With this PR new line appears at the bottom of the logs, not at the top. Dunno if you also think we should change that?

'<p>'+ message +'</p></div>');

// Scroll to top to view new messages
$('#flashMessage').scrollTop(0);
$('#flashMessage').scrollTop(1000000000);

This comment has been minimized.

Copy link
@Psycojoker

Psycojoker Jun 2, 2019

Author Member

This is kinda hackish :(

I try to find a way to calculate that easily but html/js is weird and end up losing patience.

For context: you need to pass in pixels the size up to which you want to scroll, giving a gigantic value make it scroll to the bottom.

@decentral1se
Copy link
Contributor

left a comment

Here I understand "reverse" to mean "the way all other logging works" IIUC :)

@alexAubin

This comment has been minimized.

Copy link
Member

commented Jun 7, 2019

Uuuuuh it kind of works, but when I hover the mouse on the messages - I correctly see the logs in reverse (well "normal" I guess) order - but when I unhover the mouse, the message displayed in the bar is the oldest instead of the latest ... (not sure that explanation is 100% clear @.@)

@Psycojoker

This comment has been minimized.

Copy link
Member Author

commented Jun 11, 2019

Ah, indeed >_>

Do we really care about that? I've fiddle a bit with it and couldn't find any obvious way to fix it :/

@Psycojoker Psycojoker force-pushed the inverse_log_order branch from ad172a8 to 634fe23 Jul 7, 2019

@Psycojoker

This comment has been minimized.

Copy link
Member Author

commented Jul 8, 2019

Meh.

I should have fixed it if I've identified all those horrible corner cases.

This was really painful to write.

I've written documentation in case someone is stupid enough to work on that after me.

@Psycojoker Psycojoker force-pushed the inverse_log_order branch from f0d0184 to f4e5a9a Jul 8, 2019

@alexAubin

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

Meh, I ran some tests but to me it's still not intuitive what happens ... Basically I tried to :

  • Open the admin
  • Go to the Update section to trigger some messages
  • I see the usual messages about the update stuff going on :
(info) Fetching available upgrades for system packages…
(info) Fetching available upgrades for applications…
(success) The application list yunohost has been fetched
  • So after it's done, I see "The application list yunohost has been fetched" in the top bar
  • I hover the top bar, not scrolling anything
  • I hover out
  • But now the message displayed is "Fetching available upgrades for system packages…" (the oldest) instead of the "The application list yunohost has been fetched" (i.e. the most recent)

Trying to hack the code a bit, I found that :

  • for some reason the flashMessage.scrollTop() instruction does not work ... I imagine this is related to the fact that when it's ran, the animation is still running so the bar is still not "folded" ? (Not sure tho) I was able to work around it using a setTimeout with a delay equal to 1 ... (My browser is Waterfox 56.2, dunno if that could matter)
  • most of the time (well at least when not scrolling up or anything) the code falls into an uncovered "else" case, where I would naively expect to just add a scrollTop(1000000) to go back to the very bottom (but I'm also not sure to understand the computation happening for the other cases, in diagonal it sounds related to the case where the user scrolled up ?)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.