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

Dropdowns to sort public pursuances and navigate your pursuances #111

Open
wants to merge 46 commits into
base: develop
Choose a base branch
from

Conversation

v-stickykeys
Copy link
Collaborator

I did a pass at the styling

@v-stickykeys v-stickykeys changed the title Dropdowns to sort public pursuances and navigate you pursuances Dropdowns to sort public pursuances and navigate your pursuances Feb 8, 2018
@@ -96,17 +96,30 @@ class TaskHierarchy extends Component {
)
}

produceOptions = () => {
const pursuanceArr = Object.values(this.props.pursuances);
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack Want to sort the pursuances by ID here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

produceOptions = () => {
const pursuanceArr = Object.values(this.props.pursuances);
pursuanceArr.sort((p1, p2) => {
return p1.name.localeCompare(p2.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack .toLowerCase() please :-)

orderByDateDesc = (p1, p2) => {
p1["parsedDate"] = Date.parse(p1.created);
p2["parsedDate"] = Date.parse(p2.created);
return p2.parsedDate - p1.parsedDate;
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack How about we call it created_parsed, so we know which date this is the parsed version of?

switch (action.type) {
case 'SET_PUBLIC_ORDER':
state = action.publicOrder;
return state;
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack return action.publicOrder 👍 then this is clearly a pure function, as reducers ought to be (and is!)

@elimisteve
Copy link
Contributor

@thevaleriemack Hey I tried this out and it works great! Could you just move it from the TaskHierarchy page and into the top nav bar? Then people can switch pursuances from any screen at any time, not just from the TaskHierarchy.

@v-stickykeys
Copy link
Collaborator Author

@elimisteve updated to complete #122

pursuances[currentPursuanceId] && pursuances[currentPursuanceId].name
}
</h2>
<h2 id="tasks-title">{ this.getPursuanceName(pursuances, currentPursuanceId) }</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack Looks like id should probably be pursuance-title, not tasks-title :-)

@@ -1,11 +1,14 @@
import React, { Component } from 'react';
import { Link } from 'react-router-dom';
import { Navbar, NavItem, Nav, OverlayTrigger, Tooltip } from 'react-bootstrap';
import { Nav, Navbar, NavDropdown, NavItem, OverlayTrigger, Tooltip } from 'react-bootstrap';
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack @MartyTheeMartian @Moe-Shoman How about we alphabetically order imports, as Val did here? Makes sense to me.

@@ -16,8 +19,27 @@ class NavBar extends Component {
</Tooltip>
);

showCurrentPursuance = (pursuances) => {
let id = parseInt(window.location.pathname.slice(-1), 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack You can get the pursuance ID from this.props.match.params.pursuanceId, like this:

const {
match: { params: { pursuanceId, taskGid } },
tasks,
getTasks
} = this.props;
if (!tasks.taskMap[taskGid]) {
getTasks(pursuanceId);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure how to do this. "match" is coming up undefined I think because Navbar isn't being rendered using Route

screen shot 2018-03-14 at 8 48 12 am

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can try getting PursuancePage to push its state up to Navbar? Not sure how that works with redux but I'll give it a shot.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Rendering NavBar using Route is probably the way to go, it seems to me. Luckily everything being passed to it manually is in Redux anyway, so how about reduxifying NavBar then having Route render it on every page?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Finally got this to work!

match is populated but the match is not exact because NavBar path is always root "/" so the params end up empty even when on PursuancePage.

Now I'm using location passed by react router but I still have to slice the pathname to get the id.

currentPursuanceId won't work either because it doesn't change when moving from a pursuance page to a non pursuance page like Dashboard.

screen shot 2018-03-25 at 2 26 30 pm

return p1.name.localeCompare(p2.name);
}
orderByNameDesc = (p1, p2) => {
return p2.name.localeCompare(p1.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

@thevaleriemack Please do the comparison after a .toLowerCase() on each name so that a pursuance with this name doesn't show up after This pursuance name due to capitalization differences.

onRemoveNotification={this.props.removeNotification}
onIncreaseContributionAmount={this.props.increaseContributionAmount}
/>
<Route component={NavBar} />
Copy link
Contributor

Choose a reason for hiding this comment

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

Sweet 👍 . Very clean

@elimisteve
Copy link
Contributor

Hey @thevaleriemack , I made a couple tweaks so that a hard refresh isn't required (via history.go()) and put them in a new branch -- namely https://github.com/PursuanceProject/pursuance/tree/val-dev-merged-reordered -- but I'm running into a couple CSS issues I'm having trouble with:

  1. The spacing of the elements on the top nav bar is different

  2. The top nav bar's elements are blue rather than white like in develop

  3. The Assign button's dropdown's elements are indented from the left side and also wrap around to the next line

Maybe it has something to do with a merge conflict you ran into? I don't know.

Any idea what's up (and yes I remember that you probably don't have time to fix it personally)? Thanks.

@v-stickykeys
Copy link
Collaborator Author

v-stickykeys commented Apr 25, 2018 via email

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.

None yet

2 participants