Resources breadcrumbs #97
Conversation
|
@bri616 - did this task come from a trello card? I'm just trying to make sure I understand what the code is expected to do. |
|
Yup! It was kind of an old one!
…On Sun, May 21, 2017 at 9:53 PM, bambery ***@***.***> wrote:
@bri616 <https://github.com/bri616> - did this task come from a trello
card?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#97 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGJc7IgxspGtyweCwqH-1HegixsWVnPeks5r8RTMgaJpZM4NMqCJ>
.
|
|
… On Mon, May 22, 2017 at 12:05 AM, Bri Dotson ***@***.***> wrote:
Yup! It was kind of an old one!
On Sun, May 21, 2017 at 9:53 PM, bambery ***@***.***> wrote:
> @bri616 <https://github.com/bri616> - did this task come from a trello
> card?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <#97 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AGJc7IgxspGtyweCwqH-1HegixsWVnPeks5r8RTMgaJpZM4NMqCJ>
> .
>
|
There was a problem hiding this comment.
It looks like there is no way to view a combination of Category/Tag, or multiple tags. Could it be adjusted to say something like "Viewing Category 'blah'" or "Viewing Tag 'foo'"? Also, Nathalie would prefer the breadcrumbs bar is only shown above the resource section, not above the right sidebar, if possible? Thank you! Sorry it has taken so long to review the pr :)
|
Hey @bri616 - I know this is old, but would you like to finish it up? Based on the comments that Miki posted above. Let us know? Thanks 😃 |
|
@natsteinmetz: Sure, I'm happy to dive back in! (Sorry, feedback loop on this was so slow that work on it just got lost from my list of things to do completely ❤️ ) |
|
Oh no worries at all, @bri616, it's not like we were super fast on our side either. Thanks for being willing to dive back in! 🌻 |
|
Ok, I tried to address the feedback here. It's a little weird to have breadcrumbs since we don't have a use case for it, but this code should be easy to use once we do, right now it either says "Category / selectedcategory" or "Tag / selectedtag" (not exactly what @jadefaerie wanted but it was easier to fit in the space than what was asked for.) The css changes I made are so that things will fit responsively. Here's a few screenshots:
|
|
@jadefaerie are there more changes requested here besides fixing the merge conflicts? |
|
Sorry @bri616 ! This looks good, thanks so much for doing this! |
|
Ok! I fixed the merge conflicts! |
|
I'm not sure what else to do about this MR, but I'm happy to make more changes if necessary! Just lmk. ❤️ |
|
Thanks, Bri! We're working on getting everything deployed this week! I'll
keep you posted!
…On Sun, Dec 2, 2018 at 5:23 PM Bri ***@***.***> wrote:
I'm not sure what else to do about this MR, but I'm happy to make more
changes if necessary! Just lmk. ❤️
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#97 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AZbb8yigcWW12ta763fwD4ZdoSrV4qKtks5u1H0jgaJpZM4NMqCJ>
.
|
I also added highlighting to the currently selected tag (same styling as the hover.)
One thing I noticed -- the media styling I added for the breadcrumbs is because of a problem with the responsiveness of the header in that width-range, we can probably remove it once that is fixed.
We have a fair bit of js and jquery logic (at least on this page), I'm wondering what others think about adding some js tests at some point -- or maybe moving our front-end js to something like React, too? (Which might make testing easier) .