-
Notifications
You must be signed in to change notification settings - Fork 15
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
base: develop
Are you sure you want to change the base?
Conversation
… into develop :x
@@ -96,17 +96,30 @@ class TaskHierarchy extends Component { | |||
) | |||
} | |||
|
|||
produceOptions = () => { | |||
const pursuanceArr = Object.values(this.props.pursuances); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
src/reducers/publicOrderReducer.js
Outdated
switch (action.type) { | ||
case 'SET_PUBLIC_ORDER': | ||
state = action.publicOrder; | ||
return state; |
There was a problem hiding this comment.
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!)
@thevaleriemack Hey I tried this out and it works great! Could you just move it from the |
stay up to date
… into develop stay up to date
@elimisteve updated to complete #122 |
pursuances[currentPursuanceId] && pursuances[currentPursuanceId].name | ||
} | ||
</h2> | ||
<h2 id="tasks-title">{ this.getPursuanceName(pursuances, currentPursuanceId) }</h2> |
There was a problem hiding this comment.
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
:-)
src/components/NavBar/NavBar.js
Outdated
@@ -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'; |
There was a problem hiding this comment.
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.
src/components/NavBar/NavBar.js
Outdated
@@ -16,8 +19,27 @@ class NavBar extends Component { | |||
</Tooltip> | |||
); | |||
|
|||
showCurrentPursuance = (pursuances) => { | |||
let id = parseInt(window.location.pathname.slice(-1), 10); |
There was a problem hiding this comment.
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:
pursuance/src/components/Content/Pursuance/views/DiscussView.js
Lines 16 to 24 in 6a8af04
const { | |
match: { params: { pursuanceId, taskGid } }, | |
tasks, | |
getTasks | |
} = this.props; | |
if (!tasks.taskMap[taskGid]) { | |
getTasks(pursuanceId); | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
return p1.name.localeCompare(p2.name); | ||
} | ||
orderByNameDesc = (p1, p2) => { | ||
return p2.name.localeCompare(p1.name); |
There was a problem hiding this comment.
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.
…into develop stay up to date
onRemoveNotification={this.props.removeNotification} | ||
onIncreaseContributionAmount={this.props.increaseContributionAmount} | ||
/> | ||
<Route component={NavBar} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet 👍 . Very clean
Hey @thevaleriemack , I made a couple tweaks so that a hard refresh isn't required (via
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. |
!!
I’ll look into them when I get a chance.
Nav spacing is different because I tried to employ the bootstrap nav and
center the jump to drop down.
Changing the links to white should be fine.
I don’t think I messed with the assign button. But if it was using styling
from nav css maybe that’s where the problem occurred.
On Wed, Apr 25, 2018 at 12:45 AM Steve Phillips ***@***.***> wrote:
Hey @thevaleriemack <https://github.com/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.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#111 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AIDeqktQlM7bJ6qYpwvH4gOABjUdeCInks5tsCmSgaJpZM4R94Dh>
.
--
Thank you,
Val
|
I did a pass at the styling