ResistanceCalendar / resistance-calendar-frontend

A central listing of upcoming progressive events
https://resistance-calendar-staging.herokuapp.com/
MIT License
7 stars 8 forks source link

Adds in basic naive cache of events between page transitions #180

Closed pfarnach closed 7 years ago

sdotson commented 7 years ago

Apologies if this has already been discussed, but what do you think about syncing this new eventCache with localStorage? Might be an edge case, but I think as written the cache would clear with any hard refresh.

pfarnach commented 7 years ago

That's a fair question -- as I understood the issue @pdw207 outlined, the user experience of loading a bunch of events, clicking into one, then hitting the "back to events" button was bad because the user would lose the events. Localstorage would persist between page loads which I don't think was within the scope of this issue but may very well make more sense if we want to expand what this does. If you want to take a stab at doing that, please go for it!

dorono commented 7 years ago

Looks good! However, I just pulled down this branch, and got the following error upon running yarn run dev, after running yarn. I tried going to an earlier version of node 6.4 (shot in the dark) to see if maybe it was my node version, but that didn't work.

    ERROR in ./~/css-loader?{"sourceMap":true,"modules":true,"importLoaders":2,"camelCase":true,"localIdentName":"[name]__[local]__[hash:base64:5]"}!./~/postcss-loader?{}!./~/sass-loader/lib/loader.js?{"sourceMap":true}!./src/components/EventFilters/EventFilters.sass
    Module build failed: Error: ENOENT: no such file or directory, scandir '/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/vendor'
        at Error (native)
        at Object.fs.readdirSync (fs.js:951:18)
        at Object.getInstalledBinaries (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/extensions.js:122:13)
        at foundBinariesList (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/errors.js:20:15)
        at foundBinaries (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/errors.js:15:5)
        at Object.module.exports.missingBinary (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/errors.js:45:5)
        at module.exports (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/binding.js:15:30)
        at Object.<anonymous> (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/node-sass/lib/index.js:14:35)
        at Module._compile (module.js:556:32)
        at Object.Module._extensions..js (module.js:565:10)
        at Module.load (module.js:473:32)
        at tryModuleLoad (module.js:432:12)
        at Function.Module._load (module.js:424:3)
        at Module.require (module.js:483:17)
        at require (internal/module.js:20:19)
        at Object.<anonymous> (/Users/DoronHomeFolder/Sites/resistance-calendar-frontend/node_modules/sass-loader/lib/loader.js:3:14)
        at Module._compile (module.js:556:32)
        at Object.Module._extensions..js (module.js:565:10)
        at Module.load (module.js:473:32)
        at tryModuleLoad (module.js:432:12)
        at Function.Module._load (module.js:424:3)
        at Module.require (module.js:483:17)

Any idea why I'm getting this?

dorono commented 7 years ago

OK, never mind, had to delete my yarn.lock file and it works, looking now...

dorono commented 7 years ago

@pfarnach One issue I found when doing the following: 1) Do a search that gives you more than one page-load's worth of results 2) scroll to the bottom of the events list, and click the "Load More Events" button. 3) click on an event towards the middle of the list 4) from the event detail page, click the "BACK TO EVENTS" link

Notice that you'll be taken to the bottom of the events list instead of the position in the list that you last scrolled down to.

Is this intended behavior?

pfarnach commented 7 years ago

@dorono yup, that was intended to be a temporary fix. I didn't want to complicate the cache more than it had to be (because we also need to store scroll position). I'll try to whip up something now to include in this PR if the direction we're going in looks fine

dorono commented 7 years ago

Awesome, great job!

pdw207 commented 7 years ago

@sdotson I think clearing local cache on a refresh is OK.