profile pages #407

alaq · 2018-03-21T14:45:32Z

Created separate profile page per user.
Added last three notebooks by logged in user to the top of /notebook
- If user has no notebook, message is displayed (maybe not display anything instead? no header?)
User's names are clickable and redirecting to user's profile

No error/warning in ./tools/tslint.js
This addresses #297

ry · 2018-03-21T15:02:31Z

website/nb.ts

+  async componentWillMount() {
+    // Only query firebase when in the browser.
+    // This is to avoiding calling into firebase during static HTML generation.
+    if (IS_WEB) {


Please remove the if (IS_WEB) and the comment above. The if (IS_WEB) isn't necessary (despite it being erroneously used in elsewhere in this file) this code is only executed on web now.

Ok, I went ahead and removed it from the other location in the file, if that's okay.

Yep, thanks!

ry · 2018-03-21T15:11:50Z

website/nb.ts

@@ -464,7 +480,8 @@ export class MostRecent extends Component<any, MostRecentState> {
    // This is to avoiding calling into firebase during static HTML generation.
    if (IS_WEB) {
      const latest = await db.active.queryLatest();
-      this.setState({latest});
+      const own = await db.active.queryProfile(this.props.uid);


When loading "/notebook" I get a crash here because this.props.uid is undefined.

Also, if MostRecent is going to take props, please add a MostRecentProps interface

ry · 2018-03-21T15:14:47Z

website/nb.ts

@@ -687,6 +751,19 @@ export class Notebook extends Component<NotebookProps, NotebookState> {
  }
 }

+function notebookList(notebooks: db.NbInfo[]): JSX.Element[] {


ry · 2018-03-21T15:19:01Z

website/db.ts

@@ -167,6 +168,19 @@ class DatabaseFB implements Database {
    return out.reverse();
  }

+  async queryProfile(uid): Promise<NbInfo[]> {


uid: string

ry · 2018-03-21T15:19:58Z

website/db.ts

+      const doc = snap.data();
+      out.unshift({ nbId, doc });
+    });
+    return out.reverse();


If you used push() instead of unshift(), you wouldn't have to call reverse()

ry

Fantastic!! It needs some clean up - it crashes when I load /notebook for example - but this is the first time I've been able to see my list of notebooks, so it's really adding some much needed functionality : )

alaq · 2018-03-21T19:53:27Z

I believe the changes have been addressed. Let me know.

ry · 2018-03-21T22:11:07Z

website/main.scss

+.notebook-container {
+  max-width: 960px;
+  margin: 0 auto;
+  padding: 0 8px;


This seems like an unrelated change. In fact, if we left out all the css changes completely - would it look terrible? I'd rather land the functionality first and then clean up the style later.

ry · 2018-03-21T22:11:46Z

website/nb.ts

+
+export interface ProfileState {
+  latest: db.NbInfo[];
+  profile: string;


ProfileState.profile is unused.

ry · 2018-03-21T22:12:07Z

website/nb.ts

+
+export interface ProfileProps {
+  uid: string;
+  userInfo?: db.UserInfo;


ProfileProps.userInfo is unused.

ry · 2018-03-21T22:12:56Z

website/nb.ts

+    if (!this.state.latest) {
+      return h(Loading, null);
+    }
+    const doc = this.state.latest[0].doc;


It crashes here if this.state.latest is empty. You can do something simple for now like this:

if (this.state.latest.length === 0) { return h("h1", null, "User has no notebooks"); }

ry

Looks better - not crashing and working properly. I have a few comments...

Please add a test

testBrowser(async function notebook_profileSmoke() {
  let mdb = enableMock();
  resetPage();
  let el = h(nb.Profile, { uid: "non-existant" });
  render(el, document.body);
  await Promise.resolve(); // Wait for promise queue to flush.
  let profileBlurbs = document.querySelectorAll(".profile-blurb");
  assert(profileBlurbs.length === 0);
  assert(objectsEqual(mdb.counts, { queryProfile: 1 }));

  // Try again with a real uid.
  mdb = enableMock();
  resetPage();
  el = h(nb.Profile, { uid: db.defaultOwner.uid });
  render(el, document.body);
  await Promise.resolve(); // Wait for promise queue to flush.
  profileBlurbs = document.querySelectorAll(".profile-blurb");
  assert(profileBlurbs.length === 1);
  assert(objectsEqual(mdb.counts, { queryProfile: 1 }));

});

(Feel free to expand on this / clean it up)
In the db.ts you can modify the queryProfile to do something like this:

  async queryProfile(uid: string): Promise<NbInfo[]> {
    this.inc("queryProfile");
    if (uid === defaultOwner.uid) {
      return [{ nbId: "default", doc: defaultDoc }];
    } else {
      return [];
    }
  }

ry · 2018-03-27T13:35:04Z

website/nb.ts

@@ -431,10 +431,12 @@ export class FixedCell extends Component<FixedProps, CellState> {
 export interface NotebookRootProps {
  userInfo?: db.UserInfo;
  nbId?: string;
+  uid?: string;


This is getting a bit confusing with all these ids. So "uid" is supposed to be the user id of the profile we want to display, while "userInfo" is info about the logged in user. I'd suggest changing "uid" to "profileUid". You can leave "userInfo" as is, but I will change it in the future to be "currentUserInfo".

Also, what's the purpose of having uid both in the Props and State? It seems it doesn't need to be in the props?

ry · 2018-03-27T13:39:38Z

website/nb.ts

-      this.setState({latest});
-    }
+    const latest = await db.active.queryLatest();
+    const own = await db.active.queryProfile(this.props.uid);


It's a bit unfortunate that we're doing two queries... Maybe make a comment
// TODO potentially these two queries can be combined into one.

ry · 2018-03-27T13:40:38Z

website/nb.ts

        ),
        h("div", {"class": "most-recent-header-cta"},
          h("button", { "class": "create-notebook",
                        "onClick": () => this.onCreate(),
          }, "+ New Notebook"),
        ),
      ),
-      h("ol", null, ...notebookList),
+      empty,
+      h("ol", null, ...notebookList(this.state.own.slice(0, 3))),


If you're only using 3 of the results, maybe limit the query to only return three?

Fix overflow on notebook links. Plus other visual clean ups

Now NotebookRoot makes all the queries and passes state downwards. This makes it easier to check for readiness in the tests and display errors more consistently.

alaq · 2018-03-28T15:46:06Z

Requested changes have been applied, and @ry refactoring as well.
I believe this branch is ready to be merged.

Please review.

ry · 2018-03-28T15:57:47Z

I agree, I think it's ready now.
@piscisaureus Please review

piscisaureus · 2018-03-29T13:58:01Z

The code changes look good to me.
However I think this is not rendering as intended: https://snag.gy/MOyb4N.jpg

ry · 2018-03-29T14:34:29Z

That’s as intended. I’ll let a designer make a second pass.

ry · 2018-03-29T15:46:35Z

Thank you @alaq ! This is a massive improvement to the website!

ry · 2018-03-29T16:41:40Z

Profile pages uploaded to the main site. I'm finally able to explore @qti3e's notebooks : )
http://propelml.org/notebook/?profile=nsDPpi960tUR7Y5pmbsMs0zzkN42

Again thanks for trudging thru this @alaq

alaq · 2018-03-29T16:51:50Z

My pleasure @ry
Now that I can see all of mine too, I feel like I should be able to delete the ones that I am ashamed of

ry · 2018-03-29T16:53:06Z

@alaq I had the same thought #460

ry reviewed Mar 21, 2018

View reviewed changes

alaq force-pushed the profiles branch from 555bd14 to dffeed1 Compare March 21, 2018 18:19

ry reviewed Mar 21, 2018

View reviewed changes

website/nb.ts Outdated

export interface ProfileState {

latest: db.NbInfo[];

profile: string;

Copy link

Contributor

ry Mar 21, 2018

Choose a reason for hiding this comment

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

ProfileState.profile is unused.

ry reviewed Mar 21, 2018

View reviewed changes

website/nb.ts Outdated

export interface ProfileProps {

uid: string;

userInfo?: db.UserInfo;

Copy link

Contributor

ry Mar 21, 2018

Choose a reason for hiding this comment

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

ProfileProps.userInfo is unused.

ry reviewed Mar 21, 2018

View reviewed changes

ry mentioned this pull request Mar 22, 2018

Use JSX in website/ #419

Closed

alaq force-pushed the profiles branch 3 times, most recently from fbc5829 to 0d8b11b Compare March 26, 2018 19:13

ry reviewed Mar 27, 2018

View reviewed changes

alaq force-pushed the profiles branch 3 times, most recently from de9c12b to 3581e8a Compare March 27, 2018 18:56

alaq and others added 4 commits March 27, 2018 20:12

profile pages

4218150

nb: Display titles in most-recent.

837caac

Fix overflow on notebook links. Plus other visual clean ups

Add assertObjectsEqual

1f35fca

Refactor state management in notebook.

f8850fa

Now NotebookRoot makes all the queries and passes state downwards. This makes it easier to check for readiness in the tests and display errors more consistently.

alaq force-pushed the profiles branch from 3581e8a to f8850fa Compare March 28, 2018 15:35

ry requested a review from piscisaureus March 28, 2018 15:57

piscisaureus approved these changes Mar 29, 2018

View reviewed changes

ry merged commit 6abb9cf into propelml:master Mar 29, 2018

alaq deleted the profiles branch March 29, 2018 16:51

ry mentioned this pull request Mar 29, 2018

website: on /notebook show a list of the user's own notebooks #297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

profile pages #407

profile pages #407

alaq commented Mar 21, 2018

ry Mar 21, 2018 •

edited

Loading

alaq Mar 21, 2018

ry Mar 21, 2018

ry Mar 21, 2018 •

edited

Loading

ry Mar 21, 2018 •

edited

Loading

ry Mar 21, 2018

ry Mar 21, 2018

ry left a comment

alaq commented Mar 21, 2018

ry Mar 21, 2018

ry Mar 21, 2018

ry Mar 21, 2018

ry Mar 21, 2018

ry left a comment •

edited

Loading

ry Mar 27, 2018 •

edited

Loading

ry Mar 27, 2018

ry Mar 27, 2018

alaq commented Mar 28, 2018

ry commented Mar 28, 2018 •

edited

Loading

piscisaureus commented Mar 29, 2018

ry commented Mar 29, 2018

ry commented Mar 29, 2018

ry commented Mar 29, 2018

alaq commented Mar 29, 2018

ry commented Mar 29, 2018 •

edited

Loading

profile pages #407

profile pages #407

Conversation

alaq commented Mar 21, 2018

ry Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ry Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

ry Mar 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ry left a comment

Choose a reason for hiding this comment

alaq commented Mar 21, 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

ry left a comment • edited Loading

Choose a reason for hiding this comment

ry Mar 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alaq commented Mar 28, 2018

ry commented Mar 28, 2018 • edited Loading

piscisaureus commented Mar 29, 2018

ry commented Mar 29, 2018

ry commented Mar 29, 2018

ry commented Mar 29, 2018

alaq commented Mar 29, 2018

ry commented Mar 29, 2018 • edited Loading

ry Mar 21, 2018 •

edited

Loading

ry Mar 21, 2018 •

edited

Loading

ry Mar 21, 2018 •

edited

Loading

ry left a comment •

edited

Loading

ry Mar 27, 2018 •

edited

Loading

ry commented Mar 28, 2018 •

edited

Loading

ry commented Mar 29, 2018 •

edited

Loading