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

Fix: Handle Principal IDs in the front-end #27

Merged
1 commit merged into from
Apr 20, 2020
Merged

Conversation

ghost
Copy link

@ghost ghost commented Apr 15, 2020

Overview

Introduction of Principal IDs as the id type meant we couldn't use profile.id in the front-end in the same way. This PR introduces a naive application state for LinkedUp and manages actions that way.

Changes

  • Declare an application state to store search results and connections
  • Refer to search results / connections by their index instead of profile.id
  • Prettier

Note: Prettier wreaked havoc on formatting, so I'll explicitly call out changes as comments.

@ghost ghost self-assigned this Apr 15, 2020
@@ -33,8 +33,8 @@ const main = async () => {
const profiles = require("./data");
profiles.forEach(async (profile) => {
const linkedup = await getActor("linkedup");
const userId = await linkedup.create(profile);
console.log("...profile added", userId);
await linkedup.create(profile);
Copy link
Author

Choose a reason for hiding this comment

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

create no longer returns an id so this was "logging" undefined.

connection: [],
profile: {},
results: [],
};
Copy link
Author

Choose a reason for hiding this comment

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

Here's a primitive state (to be replaced with something Redux-like?).

$(".search")
.html(searchResultsPageTmpl(results))
.show();
state.results = results;
Copy link
Author

Choose a reason for hiding this comment

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

When we get search results back, we store them in state.

try {
linkedup.connect(parseInt(userId, 10));
const profile = state.profile;
linkedup.connect(profile.id);
Copy link
Author

Choose a reason for hiding this comment

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

Connect with the currently displayed profile.

renderProfile(parseInt(userId, 10));
const showProfile = (index) => {
const profile = state.results[index];
renderProfile(profile.id);
Copy link
Author

Choose a reason for hiding this comment

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

Find the search result by index, then render it by ID.

const connectionTmpl = data => `
<a class="lu_connection" onclick="actions.showProfile('${data.id}')">
const connectionTmpl = (data, index) => `
<a class="lu_connection" onclick="actions.showProfile(${index})">
Copy link
Author

Choose a reason for hiding this comment

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

Pass the index instead of the ID.

const searchResultTmpl = data => `
<a class="lu_search-result" onclick="actions.showProfile('${data.id}')">
const searchResultTmpl = (data, index) => `
<a class="lu_search-result" onclick="actions.showProfile(${index})">
Copy link
Author

Choose a reason for hiding this comment

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

Pass the index instead of the ID.

$(".profile")
.html(ownProfilePageTmpl(data))
.show();
state.connections = data.connections;
Copy link
Author

Choose a reason for hiding this comment

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

When we load connections, put them in state.

try {
let data = await linkedup.get(userId);
state.profile = data;
Copy link
Author

Choose a reason for hiding this comment

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

When we load a profile, store it in state.

@ghost ghost requested review from hansl and lsgunnlsgunn April 15, 2020 22:07
@ghost ghost marked this pull request as ready for review April 15, 2020 22:07
@ghost ghost self-requested a review as a code owner April 15, 2020 22:07
@ghost ghost requested a review from andrewwylde April 15, 2020 22:07
Copy link
Contributor

@lsgunnlsgunn lsgunnlsgunn left a comment

Choose a reason for hiding this comment

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

Thanks for this! LGTM.

@lsgunnlsgunn
Copy link
Contributor

lsgunnlsgunn commented Apr 15, 2020

Quick test--Search result is returned and I see the Connect button, but clicking it didn't seem to do anything (or it is taking a really long time).

Refreshing the browser after doing the above pulled in the connection. Yippee ....

@ghost ghost merged commit 4cb74c8 into master Apr 20, 2020
@ghost ghost deleted the sj/frontend-principals branch April 20, 2020 16:55
This pull request was closed.
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