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

Reverse Order of Actions #38

Closed
tuckerconnelly opened this issue Jan 20, 2016 · 9 comments
Closed

Reverse Order of Actions #38

tuckerconnelly opened this issue Jan 20, 2016 · 9 comments

Comments

@tuckerconnelly
Copy link

When I open up the DevTools, I'm only really interested in the current state, not the first one.

Would be nice if the actions were reversed, so the latest one was up top.

If I get some time, I'll do a PR, but this should be super quick to do if ya'll agree :)

(BTW, this tool is freakin' awesome!!)

@zalmoxisus
Copy link
Owner

Thanks for the feedback and the suggestion!

I think solving the issue #34 would solve this as well, wouldn't it?

@tuckerconnelly
Copy link
Author

I think reverse order would be better--

Say you're up looking at an action from 10 actions ago, then boom, an action comes in and it autoscrolls you back down. Could get annoying :P

Also having the latest action at the top matches most normal "feeds" around the web (facebook feed, twitter feed, etc. all have latest stuff at the top)

@zalmoxisus
Copy link
Owner

Say you're up looking at an action from 10 actions ago, then boom, an action comes in and it autoscrolls you back down. Could get annoying :P

Actually, autoscrolling to the bottom works (the issue is only with initial rendering of the monitor), and, as you see, it doesn't bother you when you scroll to a specific action (autoscrolling happens only when you are at the bottom).

I'm neither pro nor against this proposal, but it should be advocated and implemented on the monitor side, otherwise we'll still have autoscrolling to bottom happen on the log monitor. It wouldn't be efficient and sustainable to revert actions' array and states' object every time an action is dispatched, way better to just revert the loop. Also I want the extension behavior to be the same as Redux DevTools, so one could easy switch from the classical implementation of Redux DevTools to the extension and back.

@gaearon
Copy link

gaearon commented Jan 29, 2016

The log monitor should already autoscroll if you didn't scroll up manually.

@zalmoxisus
Copy link
Owner

@gaearon, yes, it does. The issue is with the initial rendering when having already actions stored in the liftedStore. We should have triggered that also for componentDidMount.

@gaearon
Copy link

gaearon commented Jan 29, 2016

Ah right. I'm happy to accept a PR that scrolls to bottom in componentDidMount() if the scrollTop received via props is 0.

@zalmoxisus
Copy link
Owner

@gaearon, I'm afraid it could be a problem when using persistState. Let's say, I scrolled to the top, refreshed the page and the monitor autoscrolled to the bottom, which wouldn't be the expected result. So, we have to have autoscroll on mounting only when preserveScrollTop is false. Anyway, we don't need preserveScrollTop in the extension, otherwise it will autoscroll monitors in different windows synchroniously.

@zalmoxisus
Copy link
Owner

Should work in v1.0.0-beta3.

Also the idea of reversed actions for the notifications is good and, actually, implemented in Diff Monitor. We could add this for the popup page to make the actions look as notifications (with what was changed) and to "match most of feeds around the web".

@zalmoxisus
Copy link
Owner

In v1.3.0 now it's possible now to select monitors directly in the monitor window, Chrome devtools panel or browser action popup. So it's up to you to add / use any custom monitors. Monitors are moved to remotedev-app, as it's just a simple web app, it's easier to add and test custom monitors. It's just one file to be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants