Skip to content
This repository has been archived by the owner on Feb 22, 2020. It is now read-only.

Refactor imagery loading EventEmitter into store value #305

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

louisgv
Copy link
Contributor

@louisgv louisgv commented Jan 31, 2018

Description

I refactored the event emitter inside the TreeView to send it to the global store. This way if any other component need access to the imagery, it can simply grab it from the store.

I also fixed an UX problem with the imagery list expanding button as well as the startcase button, making them more visible and not in the way with the image. (Screenshot below)

Reference to official issue

#302
#295

Screenshot

image

How Has This Been Tested?

Manual testing

CLA

  • I have signed the CLA; if other committers are in the commit history, they have signed the CLA as well

@lamby
Copy link
Contributor

lamby commented Jan 31, 2018

Please rename this PR (namely, remove "[WIP] " suffix!) when we should review in more detail, or ping here for review ahead of that :)

@louisgv louisgv changed the title [WIP] Refactor imagery loading EventEmitter into store value Refactor imagery loading EventEmitter into store value Jan 31, 2018
@louisgv
Copy link
Contributor Author

louisgv commented Jan 31, 2018

@lamby Oops, forgot to remove that. Please review!

@lamby
Copy link
Contributor

lamby commented Feb 1, 2018

LGTM :)

@louisgv
Copy link
Contributor Author

louisgv commented Feb 1, 2018

@lamby should I rebase/repush in order to make it pass Travis?

@reubano
Copy link
Contributor

reubano commented Feb 1, 2018

If you haven't already, then yes, please rebase.

@louisgv louisgv force-pushed the GH302-refactor-event-emitter branch from 3a5af43 to 0663ae3 Compare February 1, 2018 22:16
@louisgv
Copy link
Contributor Author

louisgv commented Feb 1, 2018

@reubano Done

@reubano
Copy link
Contributor

reubano commented Feb 6, 2018

When trying to run this on my own, I keep getting a blank screen at port 8080 and the following error:

GET http://34.227.31.217:8080/app.js net::ERR_CONTENT_LENGTH_MISMATCH

Anyone else? FYI, it's not specific to this PR either. I see even at 666fb1.

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

Successfully merging this pull request may close these issues.

3 participants