Data Browser #36

subdavis · 2018-10-05T14:03:09Z

fixes #31

subdavis · 2018-10-05T14:09:46Z

I'm having a bit of trouble with balancing features with code efficiency and complexity, so I wanted to ask a couple questions for what features are important:

The main model is a location prop that determines where the browser's initial position is. It has fields id, type which correspond to a girder Id and one of collection, user, folder.

What is a reasonable set of features for the consumer to have? I think it should be able to:

Specify the initial location for the browser
Maintain the model for location, such that:
- it can intercept or block or block location changes
- it can change the browser location at any time.

If this is the case, FileBrowser will need to build the breadcrumb backwards with iterative api calls.

I also set some constraints:

Don't use watched properties
Don't use mounted or created for anything other than setting up event listeners.
Use async computed for any properties that rely on server data (credit @jbeezley - I really like this library)
Don't make unnecessary API queries.

I'm running into an issue with re-computation of the breadcrumb. I could fairly easily do what I want by caching all the folder paths I've already seen, and only issuing an API call when I arrive at a parent folder I don't already have the details for. This is, however, a little messy.

zachmullen · 2018-10-05T14:11:18Z

src/components/Authentication/OAuth.vue

@@ -17,7 +17,7 @@ const iconMap = {
  linkedin: { dark: true, icon: 'linkedin', color: '#283e4a' },
  bitbucket: { dark: false, icon: 'bitbucket' },
  box: { dark: true, icon: 'box_com', color: '#0071f7' },
-  globus: { dark: true, icon: 'globus', color: '#335a95' },
+  globus: { dark: true, icon: 'globe', color: '#335a95' },


We should isolate this in its own PR since it's unrelated to the topic.

I'm using the globe icon in the file browser, so I changed its name to something generic. Should I create a duplicate icon and then remove the duplicate in another PR?

Ah, I see. I'd actually like to keep the globus icon identifier separate from globe, even though they would point to the same thing right now. One is a brand and one is generic, so it makes sense to have two different semantic identifiers for them.

jeffbaumes · 2018-10-05T15:52:14Z

src/components/FileBrowser/FileBrowser.vue

+      th(width="1%").pr-0
+        v-btn(flat, small, v-if="newFolderEnabled")
+          v-icon.mdi-24px.mr-1(left, color="accent") {{  $vuetify.icons.folderNew }}
+          | NEW FOLDER


I believe v-btn will automatically capitalize text in buttons. I'd prefer we keep them "Sentence case" in markup. The upcoming iteration of material design seems to remove the idea of uppercasing everything so it is a forward-compatible practice.

jeffbaumes · 2018-10-05T15:53:35Z

src/components/FileBrowser/FileBrowser.vue

@@ -0,0 +1,252 @@
+<script>


Interesting to do JS first, then template, then CSS. I don't mind this, but we should standardize on an ordering in Vue files for consistency.

I picked this up at our local DC Vue meetup group.

The justification is that you always look at (html, script) or (html, css) but never (script, css), so it makes more sense to have your adjacency like this.

I have been putting template, script, then style. but script, template, style actually feels better based on the frequency of the use.
I learned a few style from the official style guide, https://vuejs.org/v2/style-guide/#Single-file-component-top-level-element-order-recommended

I'll make a PR to add plugin:vue/recommended to our eslint extensions.

#37 does this, but doesn't appear to include enforcement of the top level tag ordering... will look into this further after lunch.

jeffbaumes · 2018-10-05T16:01:39Z

The breadcrumb is always annoying to think about, and there are odd edge cases (e.g. where the user does not have access to every folder up the chain to a user/collection).

Don't make unnecessary API queries.

I personally wouldn't worry about caching and would relax this constraint. Code clarity probably trumps efficiency at this point in the development of these components.

subdavis

Brief comments. More to come, probably.

subdavis · 2018-10-09T19:11:47Z

src/App.vue

+    girder-upload(v-if="uploader",
+        :dest="uploadDest",
+        multiple="multiple",
+        @done="refresh++")


I don't especially like this, but I also don't see a way around it. In order to force computed properties to re-compute without any of their dependencies having changed where the trigger is an event from Upload.vue, this seems like the best way to do it.

On the plus side, if the client wanted to implement some kind of polling loop to keep the file browser up to date, this would make it trivial.

https://forum.vuejs.org/t/force-computed-properties-to-recompute/2769

Not sure if this would also solve the problem, but this at least seems a little more simple and explicit (calling this.$forceUpdate() in one place seems more readable than having a mysterious refresh property that is mentioned in 5 places in the .vue file).

@jeffbaumes I tried that first. As far as I can tell, calling $forceUpdate() doesn't have any effect. I tried calling it with $refs from the parent and calling a function in the child that then calls this.$forceUpdate() but the computed properties don't re-compute.

EDIT: Poked around and Evan confirms this: vuejs/vue#7395 (comment)

Looks like there is actually no way to do this without an explicit dependency update?

Here is an officially sanctioned way to deal with this (which matches your approach well enough):

vuejs/vue#214 (comment)

However, I believe there is a more fundamental issue going on. It feels like using asyncComputed tarnishes the purity of computed properties. Computed properties are automagic and work because they are only supposed to depend on normal data properties. asyncComputed properties by definition depend on "other stuff", like server calls which actually hold the state, so we may find ourselves often bumping up against this impurity when using asyncComputed. It could be argued that we should not be using asyncComputed, but should have normal data properties for things like count and rows, and methods that mutate them with async code. That way, we would just call the appropriate method(s) when the upload completes, and things will reactively update as normal.

I think you may be right about asyncComputed. I do, however, like the organization (particularly the ease of forming a mental model of the dependency tree) it affords you. @jbeezley do you have any insight about whether or not asyncComputed is appropriate in this case, since you've spent a lot more time with it than me?

I'll make a new issue for this discussion so it doesn't get lost in the noise of this PR.

subdavis · 2018-10-09T19:40:20Z

src/components/FileBrowser.vue

+    },
+  },
+  computed: {
+    folderParams() {


think I should maybe roll this and itemParams into row since it's the only function that calls them.

Might make the vue computed property jitter less bouncy to have them all in one function too.

jbeezley · 2018-10-10T13:55:45Z

src/components/FileBrowser.vue

+          const resp = await gr.get(endpoint, { params });
+          if (resp.status !== 200) return [];
+          return resp.data;
+        };


Avoid nested functions when possible.

I agree with Jon. This logic unnecessary and is swallowing exceptions. I think I can just have a plain request and let it fail fast.

jbeezley · 2018-10-10T13:57:54Z

src/components/FileBrowser.vue

+          this.counts,
+          this.location,
+          this.refresh,
+        ];


Do you need watch all of these explicitly? Unless there is something I don't understand this.refresh should be the only value that needs to be here.

I've found that this de-bounces the async computed properties quite a bit. With these lines, the app produces many fewer invalid rest requests while the state settles. It serves to flatten out what was a pretty deep dependency tree.

It also prevents a race condition where the value of counts returns too late so a previous value is used and rows ends up containing more rows than pagination allows.

Hi @subdavis, I gave this PR a try, it looks like because of itemParams changes, each time I navigate to a folder, two very similar requests are sent. So the debouncing you mentioned may not be fully working.
I myself also confused why there are so many dependencies. Theoretically, I think a computed should only change when it's dependencies change, so if the rows is not depending on counts, location, or refreshCounter_, it shouldn't be computed again when those change.

jbeezley · 2018-10-10T13:59:38Z

src/components/FileBrowser.vue

+      }
+    },
+    selected(newval) {
+      this.$emit('update:selected', newval);


I would avoid the update:... event name unless it is actually a sync'ed prop.

jbeezley · 2018-10-10T14:04:40Z

src/components/FileBrowser.vue

+    },
+    requireSession() {
+      if (!this.login || !this.location) {
+        throw new Error('File Browser expects an active session and a defined location.');


We should leave open the possibility of browsing anonymously. As for the location prop, we can do a prop validator rather than checking it before every rest call.

jbeezley · 2018-10-10T14:15:51Z

src/App.vue

+    },
+  },
+  methods: {
+    initializeLocation() {


Maybe call this resetLocation because it's not just called at initialization.

matthewma7 · 2018-10-15T17:49:23Z

src/components/FileBrowser.vue

+      };
+    },
+    itemParams() {
+      const limit = this.counts.nFolders > this.folderParams.offset


I didn't fully understand the logic of the limit calculation, so maybe we should add some comments?

matthewma7 · 2018-10-15T17:52:14Z

src/components/FileBrowser.vue

+        offset: (this.pagination.page - 1) * this.pagination.rowsPerPage,
+      };
+    },
+    itemParams() {


Overall, the itemParams and folderParams are only being used by the rows computed, in addition to the duplicate request behavior I mentioned in another comment, I am thinking maybe we should consider reconstruct the logic and make them simply local variables inside the rows computed, instead of two explicit computed.

matthewma7

Hi @subdavis and others, I added some review to this PR. Let me know if they make sense.

src/components/FileBrowser.vue

matthewma7 · 2018-10-15T18:14:02Z

src/components/FileBrowser.vue

+        this.rowsLoading = true;
+        const rows = [
+          ...(await request(this.girderRest, GIRDER_FOLDER_ENDPOINT, this.folderParams)),
+          ...(await request(this.girderRest, GIRDER_ITEM_ENDOINT, this.itemParams)),


I think this is a place we should use Promise.all([]) instead of two await because following awaits won't be executed until the former ones finish, losing the async concurrency that we should have.

matthewma7 · 2018-10-15T18:15:52Z

src/components/FileBrowser.vue

+        v-checkbox.secondary--text.text--darken-1.pr-2(
+            :input-value="props.selected", accent, hide-details)
+      td.pl-1
+        span.text-container.secondary--text.text--darken-3(


I didn't find a clean solution excepted for adding line-height to make the text and icon better aligned.

matthewma7 · 2018-10-15T18:17:26Z

src/components/FileBrowser.vue

+    :pagination.sync="pagination",
+    :items="rows",
+    :total-items="totalItems",
+    :loading="loading ? 'accent' : false",


This one is totally on Vuetify. But maybe we could overwrite the style so that the loading indicator doesn't push the content down and up when being shown and invisible.

I looked into that. Because of where it's placed in the table (as a tr), there's just no way to do this that I have found. I think i can put my own loading bar into the table header then position it absolutely to keep it from taking up space on the page.

I'm actually pretty miffed that Vuetify would do something this silly.

It was not straight forward, but seems adding

.girder-file-browser-component .v-datatable__progress .v-progress-linear position absolute

Would do the trick

matthewma7 · 2018-10-17T14:04:23Z

src/components/FileBrowser.vue

+        ];
+      },
+    },
+    rows: {


@subdavis Thanks for updating this rows() and the comment on limit calculation. @jbeezley @zachmullen It seems the doubled fetch still exists and its caused by vue-async-computed. The rows() depends on counts(), and it's being evaluated once before counts() updates and once after counts() updates. I created a jsfiddle to illustrate the probelm. I have been using vue-async-computed in other projects and I like it, but it seems this is a case where normal data() + watch:{} is better suited.
#39

matthewma7 · 2018-10-24T15:11:31Z

src/components/FileBrowser.vue

+</template>
+
+<style lang="stylus" scoped>
+@import '~vuetify/src/stylus/settings/_colors.styl'


Hi @subdavis, I just noticed that you are importing Vuetify stylus in this PR. But the recent master change to SCSS will prevent you from doing this after rebasing master. But at the same time, I think switching to JavaScript color is better because using JavaScript color will make it adaptive to individual app theme and, as Vuetify document states, using stylus color will increase CSS export by~30kb.

subdavis · 2018-10-24T15:59:55Z

src/components/FileBrowser.vue

+  .v-table tr
+    &.itemRow
+      &[active], &:hover
+        background: $light-blue.lighten-5 !important


Hey @matthewma7 -- I did this here only because I needed to override color on a tr with !important because the theme colors aren't respected by Vueitfy for background active and hover row colors.

As far as I'm aware, there isn't a way to inline pseudo-selectors like :hover, so setting the hover and active background to blue can only be done from the stylesheet, where JS colors aren't available.

After rebasing, I think this color import will still be necessary. Does that sound right, or is there another way?

@subdavis Ah, I see. This is tricky. I was thinking we should always use javascript color to support theming out of box, but seems there are some things I failed to consider. Maybe since Vuetify is moving to SCSS and this usage is static, we just copy the value of $light-blue.lighten-5 for now?

Works for me! Thanks .

Cool, thanks 🙂

jeffbaumes · 2018-10-31T18:44:39Z

@subdavis: @jtomeck would like to try some styling changes, but this is still using stylus. Can you merge in master and update styles to SCSS before he works on styling?

subdavis · 2018-10-31T20:13:38Z

Sure thing. @jeffbaumes @jtomeck rebased and changed to scss.

subdavis · 2018-11-16T14:23:00Z

could we rename this component to "DataBrowser"

Yes! I have a feeling I'll need to factor the data table part out of DataBrowser to make a component that can display an arbitrary list of items/folders to suite the "Search Results" view in #40 but we will cross that bridge when we get to it.

My vote would be that we do not expose [Breadcrumb] in index.js

Agreed.

tests/unit/utils.js

zachmullen · 2018-11-16T19:38:04Z

src/components/Breadcrumb.vue

+<style lang="scss">
+.girder-breadcrumb-component {
+  &.v-breadcrumbs li:nth-child(2n) {
+    padding: 0 4px !important;


I'm still in favor of removing this rule, I don't think the padding is important enough for us to be reaching deep into Vuetify internals to change it. I think our best bet would be to open an issue request (or even a PR!) against Vuetify to allow a dense prop on the breadcrumb component, or make it so that the divider slot is in charge of its own padding (which may be harder to pitch to them since it's breaking).

I'd prefer not to have this as well, but I find the spacing between dividers (12px on either side) to be unreasonably large. The actual problem isn't the padding -- it's the mdi-chevron icon which makes really poor use of its vector box - it has another 12px between the wall of the image box and the start of the icon, making the gap look like ~24px to either side.

I don't think this is a large enough use case for the Vuetify maintainers to accept a dense prop as compensation for an abnormal icon, and fixing the icon doesn't seem like a great option either.

What about if we just use a / or a unicode chevron?

Here it is with both. Let me know which one you prefer -- I think both look fine, but I kinda prefer /

I like the slash too, and it also will be familiarly associated with the filesystem separator.

zachmullen · 2018-11-16T19:46:45Z

src/components/Breadcrumb.vue

+  v-icon.mdi-24px(
+      :disabled="disabled",
+      slot="divider",
+      color="accent") {{ $vuetify.icons.chevron }}


The "accent" color makes these dividers look like links, which they aren't. The default is that they are grey, which I think I like better. @jtomeck are you OK to change the color?

@zachmullen I don't know that it's super important for me to push back on because it really just comes down to opinion, so if you really think it's needed I'm fine with changing the color. I'll give my opinion on it anyway though in case anyone's interested :P

From a designer's perspective it seems like many people are of the mindset that giving something a color makes it look like a link. I think it all depends on the usage. For example, if I had a block of copy and changed the color of a few words, sure they would look like a link. I don't think things like giving a prominent headline a color, or giving dividers a color make them look like links, especially since nothing happens on hover and the cursor does not change.

zachmullen · 2018-11-16T19:52:55Z

Right now the "select" (click the row without clicking the name) causes the checkbox to be checked. I think selecting and checking a row should be independent actions.

subdavis · 2018-11-16T19:57:20Z

think selecting and checking a row should be independent actions

Could you link me to an example of where a data table behaves like this in the wild? I'm having trouble understanding the difference between select and check.

Do you mean that there should be 2 events published by the DataBrowser? Independent lists for "selected" and "checked"? What is the difference in purpose between select and check (i.e. when would I want to do one vs. the other?)

zachmullen · 2018-11-16T20:00:20Z

GMail is one example, clicking the row actually changes the route, not check the box. But in our case, there are really 3 different actions people want to take on a row:

Drill down into that resource (i.e. descend the hierarchy)
Check some items (for batch operations)
Select a resource or multiple resource (e.g. "select a folder", in which case we wouldn't render the checkbox at all).

I could be wrong about this behavior, curious what @jtomeck thinks

subdavis · 2018-11-16T20:49:32Z

Updated to use / as separator. I'm aware that this is the default separator, and leaving this slot empty would look almost the same. But the subheading style makes / noticeably bigger, which makes the bottom of the character extend below the baseline and looks a lot better to me.

Here it is without the slot:

I think we should keep the custom styled /.

zachmullen · 2018-11-16T20:50:46Z

That's fair. It's worth mentioning that the color="accent" on the divider element no longer has any effect, so we can remove it.

subdavis · 2018-11-16T21:00:21Z

I may be getting too personally invested in the minutia of DataBrowser. We've been together for so long now. I nearly fell on my sword over the height of a /.

We can totally remove the custom slot.

jeffbaumes · 2018-11-16T21:04:13Z

@subdavis we as a team/company tend to underestimate the importance of good clean design, so please don't feel you are being overly picky. I'm also not saying we should keep the custom slot because there is always a maintainability tradeoff, but just know that attention to design detail is generally something we should strive to improve.

jtomeck · 2018-11-16T21:18:36Z

Right now the "select" (click the row without clicking the name) causes the checkbox to be checked. I think selecting and checking a row should be independent actions.

I agree with this ^

Drill down into that resource (i.e. descend the hierarchy)

Check some items (for batch operations)

Select a resource or multiple resource (e.g. "select a folder", in which case we wouldn't render the checkbox at all).

As well as this ^

jtomeck · 2018-11-16T21:19:45Z

I may be getting too personally invested in the minutia of DataBrowser. We've been together for so long now. I nearly fell on my sword over the height of a /.

Now you know the plight of the designer @subdavis haha

zachmullen

Just one nit, and the removal of no-await-in-loop, then this LGTM 👏

zachmullen · 2018-11-16T21:23:58Z

src/components/DataBrowser.vue

+            :class="{selectable: props.item.type !== 'item'}",
+            @click.stop="changeLocation(props.item)")
+          v-icon(:color="props.selected ? 'accent' : ''") {{ $vuetify.icons[props.item.icon] }}
+          span &nbsp; {{ props.item.name }}


  -> .pl-1 or something

subdavis · 2018-11-19T14:38:47Z

removal of no-await-in-loop

Done. Thought I did that already...

  -> .pl-1 or something

👍

Right now the "select" (click the row without clicking the name) causes the checkbox to be checked. I think selecting and checking a row should be independent actions.

I gave this a try. Requiring the checkbox click for check makes sense, but having any other part of the row trigger select felt odd. I could click any part of the row and navigate into a subfolder, meaning a stray click anywhere on the table has the potential to disorient a user: "Hey, what changed? What did I do?"

Should this table behave like the current Girder browser, where a stray click on some part of the table that isn't an actionable item simply do nothing?

BTW: I modeled my original behavior on this table from the Vuetify docs.

zachmullen · 2018-11-19T14:48:38Z

Barring any objections, I think we should merge this. If there are any lingering issues, let's open tickets for them. A couple things that might merit issues:

Multiple requests getting sent unnecessarily... was this resolved on this branch or is it still an issue?
Check vs. select actions

subdavis · 2018-11-19T14:52:19Z

Multiple requests getting sent unnecessarily... was this resolved on this branch or is it still an issue?

This was resolved thanks to guidance from @matthewma7

Check vs. select actions

Opened #51

zachmullen · 2018-11-19T14:53:04Z

subdavis added the enhancement New feature or request label Oct 5, 2018

zachmullen reviewed Oct 5, 2018

View reviewed changes

jeffbaumes reviewed Oct 5, 2018

View reviewed changes

subdavis force-pushed the browser branch from b20ae98 to 5820647 Compare October 9, 2018 18:25

subdavis commented Oct 9, 2018

View reviewed changes

jbeezley reviewed Oct 10, 2018

View reviewed changes

jbeezley mentioned this pull request Oct 10, 2018

Reconsidering the use of vue-async-computed #39

Closed

matthewma7 reviewed Oct 15, 2018

View reviewed changes

matthewma7 reviewed Oct 17, 2018

View reviewed changes

matthewma7 reviewed Oct 24, 2018

View reviewed changes

subdavis commented Oct 24, 2018

View reviewed changes

subdavis and others added 10 commits October 31, 2018 15:55

WIP

d652df2

WIP

cd75812

Autolint

d591961

Simplify app loading

1608713

Reverr reference to rig

aac2a75

Respond to comments, internalize refresh counter

5a1f354

Bug: type => _modelType

451e011

Refactor row calculation a bit

17637cb

Linting

d8adcb2

Switch to SCSS

64ec42a

subdavis force-pushed the browser branch from ef01484 to 64ec42a Compare October 31, 2018 20:13

Linting fixes

5a6b7d3

Rename to DataBrowser

7b0efa8

subdavis changed the title ~~File Browser~~ Data Browser Nov 16, 2018

zachmullen reviewed Nov 16, 2018

View reviewed changes

tests/unit/utils.js Show resolved Hide resolved

zachmullen reviewed Nov 16, 2018

View reviewed changes

Use / as separator

c33ae5b

subdavis added 2 commits November 16, 2018 15:51

remove accent

4c12b64

collapse line

34de237

zachmullen requested changes Nov 16, 2018

View reviewed changes

Nitfix

e55c6dc

pug lint fix

4f4d843

zachmullen approved these changes Nov 19, 2018

View reviewed changes

subdavis mentioned this pull request Nov 19, 2018

Check vs Select action #51

Open

subdavis merged commit dc8ab20 into master Nov 19, 2018

subdavis mentioned this pull request Nov 19, 2018

Using createLocalVue in testing causes errors #32

Closed

subdavis deleted the browser branch February 7, 2019 21:16

Data Browser #36

Data Browser #36

Conversation

subdavis commented Oct 5, 2018

subdavis commented Oct 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffbaumes commented Oct 5, 2018

subdavis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subdavis Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subdavis Oct 10, 2018 • edited Loading

Choose a reason for hiding this comment

matthewma7 Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewma7 Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewma7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subdavis Oct 15, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matthewma7 Oct 17, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subdavis Oct 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffbaumes commented Oct 31, 2018

subdavis commented Oct 31, 2018

subdavis commented Nov 16, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zachmullen commented Nov 16, 2018

subdavis commented Nov 16, 2018

zachmullen commented Nov 16, 2018 • edited Loading

subdavis commented Nov 16, 2018

zachmullen commented Nov 16, 2018

subdavis commented Nov 16, 2018

jeffbaumes commented Nov 16, 2018

jtomeck commented Nov 16, 2018

jtomeck commented Nov 16, 2018

zachmullen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subdavis commented Nov 19, 2018

zachmullen commented Nov 19, 2018

subdavis commented Nov 19, 2018

zachmullen commented Nov 19, 2018

subdavis commented Oct 5, 2018 •

edited

Loading

subdavis Oct 10, 2018 •

edited

Loading

subdavis Oct 10, 2018 •

edited

Loading

matthewma7 Oct 15, 2018 •

edited

Loading

matthewma7 Oct 15, 2018 •

edited

Loading

subdavis Oct 15, 2018 •

edited

Loading

matthewma7 Oct 17, 2018 •

edited

Loading

subdavis Oct 24, 2018 •

edited

Loading

zachmullen commented Nov 16, 2018 •

edited

Loading