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

Refactor online system + personal records #207

Merged
merged 32 commits into from
Nov 3, 2015
Merged

Refactor online system + personal records #207

merged 32 commits into from
Nov 3, 2015

Conversation

dtinth
Copy link
Member

@dtinth dtinth commented Oct 4, 2015

Right now the online system code is a spaghetti.

In order to implement more advanced features, such as, fetching the record from the cloud server, and stuff like that… I first need to clean up the existing code.

THIS PR IS GETTING BIG

The scope of this PR, so let’s define the scope:

  • Refactor online system
  • Display personal records in the music list

OUT OF SCOPE DON"T DO IT

  • Display personal record in the info tab (that go to separate PR)

@@ -15,6 +19,9 @@ export default React.createClass({
song.tutorial
? <div className="MusicListItemのtutorial">Tutorial</div>
: <div className="MusicListItemのinfo">
<div className="MusicListItemのcharts">
{this.renderChartlist(song.charts)}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderMusicListItemCharts

Because of weird bug where nested flexbox gets an inferred flex-basis of 0. Therefore, resorting back to use floats.
[getRecord川], () => doGetRecord川,
[unauthenticated川], () => doGetUnauthenticated川
)
let operationTransition川 = operation川.flatMapLatest(performWithOperation)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do it without operationTransition川

@dtinth
Copy link
Member Author

dtinth commented Oct 9, 2015

NEED MORE TEST COVERAGE! MOCK!! STUB!!! MASH!!!! SMASH!!!!! BASH!!!!!! AAAHHAHHAHAHAHHHHHH!!!!!!!

@dtinth dtinth changed the title Refactor online system. Refactor online system + personal records Oct 9, 2015
@dtinth
Copy link
Member Author

dtinth commented Oct 9, 2015

Problem with this implementation:

  • Rank is not fetched when viewing the scoreboard if the user’s personal record is already fetched.

  • Record is not cleared when logging out.

  • Record not reloaded when signing in…

    This is because we don’t have a notion of “observing” a chart’s record. To do this we need to do some refcounting, e.g., subscribe+unsubscribe API.

Looking at structuring our data, maybe we can make it so that records store is split into separate store for each user and not clear them at all…

@dtinth
Copy link
Member Author

dtinth commented Oct 10, 2015

A cleaner architecture may be like this:

artboard 1

  • User: Holds model about current user.
  • Scoreboard: Holds the scoreboard of every chart.
  • Record: Holds the record of every user-chart tuple—er—I mean, combination.
  • Online: Composes them into “declarative feng shui bacon”

dtinth added a commit that referenced this pull request Nov 3, 2015
Refactor online system + personal records
@dtinth dtinth merged commit f8dc7ac into master Nov 3, 2015
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

Successfully merging this pull request may close these issues.

1 participant