Quicclinks page, ready to go! (Also fixed sidebar bug) #50
Conversation
remove DetailContainer, DetailColumn, DetailAnswer from Faq (import)
|
Do you mind quickly fixing #44 too? |
|
@jackyzha0 ya i was just about to jump on that rn ;) you want it on this PR? |
src/components/Quicklinks.js
Outdated
|
|
||
| return ( | ||
| <> | ||
| {createQuicklinksBlocks(categories)} |
There was a problem hiding this comment.
might be good to just inline this function, don't see the benefit of just pulling it out if it just renders the component
There was a problem hiding this comment.
Oop good catch, I didn't notice!
src/containers/Quicklinks.js
Outdated
| @@ -71,19 +71,42 @@ export const QuickLinks = () => { | |||
| }) | |||
| .then(links => { | |||
| //TODO: Group links into categories | |||
| console.log(links) | |||
| setLinks(links) | |||
| const dummyLinks = [ | |||
There was a problem hiding this comment.
Yeah, this looks good! Don't forget to make a PR for this on https://github.com/nwplus/database-schemas/ to update the quick link schema.
After that's done, we can transition this to pull the data from Firebase
There was a problem hiding this comment.
Cool, good reminder, thanks!
There was a problem hiding this comment.
Ohp I think the schema was already good! I've removed the dummy links now that we know the schema will work with the code 👍
yeah, it should be pretty small so feel free to include it here |
|
@jackyzha0 FYI i fixed #44 by overriding borders (ie removing them) from the styled A in the sidebar instead of directly at |
acchiang
changed the title
src/components/Common.js
Outdated
| flex-basis: 100%; | ||
| flex: 1; |
There was a problem hiding this comment.
| flex-basis: 100%; | |
| flex: 1; | |
| flex: 1 100%; |
ehehhH https://developer.mozilla.org/en-US/docs/Web/CSS/flex
There was a problem hiding this comment.
mmm this doesn't work for some reason, i think it's because when flex takes 2 variables it doesn't treat the second one as flex-basis
There was a problem hiding this comment.
we don't want this, right?
There was a problem hiding this comment.
Oh, maybe flex basis 100% is getting overwritten by the shorthand right after it? (Can we remove it?)
When omitted from the flex shorthand, its specified value is the length zero
There was a problem hiding this comment.
done! it just says flex: 1 now
src/components/QuicklinksCard.js
Outdated
| <H2>{title}</H2> | ||
| <UL> | ||
| { | ||
| links.map((link) => | ||
| <LI><A href={link.href}>{link.label}</A></LI> | ||
| // TODO: "key" prop must be unique, else error in console (nbd imo? -AC) |
There was a problem hiding this comment.
Hopefully link.label is unique for each link, I think we can remove this comment
There was a problem hiding this comment.
okay sounds good! i added this bc there's no guard anywhere for unique hrefs but i'm sure it'll be fine in practice ;)
There was a problem hiding this comment.
Were you getting console warnings/errors ?
There was a problem hiding this comment.
oh i got console errors with the dummy links because there were two google.coms but not anymore since we'll be using links from firebase from now on
|
lmAO @jackyzha0 SORRY - one more time ples |
|
thamk u all |
Edit: removed comment about dummy links data since they're not needed anymore :)