FormidableLabs / victory-docs

Documentation for Victory: A collection of composable React components for building interactive data visualizations
http://formidable.com/open-source/victory
30 stars 64 forks source link

Refactor sidebar components #658

Closed BrittanyIRL closed 4 years ago

BrittanyIRL commented 4 years ago

My goal here was to make this a little easier to read and update to use modern React structure w/ hooks and components.

To test:

If you see anything that should be included here let me know, i think i stared at the table of contents stuff too long.

BrittanyIRL commented 4 years ago

(I'll add reviewers to this once i hunt down what's breaking the build. There's something going wrong with the gallery item template - it's trying to filter undefined. I didn't mess with the gallery so i'm just trying to retrace steps to see what went wrong with the update i did do.)

boygirl commented 4 years ago

@BrittanyIRL I'm noticing a few regressions in this branch

boygirl commented 4 years ago

Since you're working with the sidebar, it would be great if you could update the logo while you're at it.

BrittanyIRL commented 4 years ago

@BrittanyIRL I'm noticing a few regressions in this branch

  • The guides section no longer shows up in the sidebar at all
  • search is behaving quite strangely. It looks like it might only be searching props. Even then, there's some additional odd behavior:

@boygirl - Sorry about that! I had thought the guides/faq weren't supposed to be there anymore since they had been gone as far as I was aware while working on different things here. They should be back. @wgolledge was spot on about the search so that should be working as a mirror to prod now from what I can tell.

I will update the logo separately if that's ok.

could you take a second look when you get time?

boygirl commented 4 years ago

@BrittanyIRL This is looking great. Just one minor change left. Victory Guides should not be a separate url or exist as an element in the sidebar. Currently this page only exists at /guides. Actually, what do you think about nuking that page all together along with the link to /guides from the home page?

BrittanyIRL commented 4 years ago

@BrittanyIRL This is looking great. Just one minor change left. Victory Guides should not be a separate url or exist as an element in the sidebar. Currently this page only exists at /guides. Actually, what do you think about nuking that page all together along with the link to /guides from the home page?

i was confused by this page's existence and had made this task to handle it, so thank you for bringing this up!

I think axing it would be good - it's just an overview of the other content in this section anyway and you can't actually get to it by clicking on the 'guides' section since that's just supposed be an indicator of the nested items.

BrittanyIRL commented 4 years ago

approved pending removal of Victory Guides page from sidebar

It's gone - which honestly i think was a great idea, none of the other sections of the sidebar have an 'index' file with an overview, i think this makes things more consistent.

Screen Shot 2020-02-20 at 1 44 46 PM
boygirl commented 4 years ago

@BrittanyIRL great! Please do get rid of the guides link here as well, then: Screen Shot 2020-02-20 at 1 20 33 PM

BrittanyIRL commented 4 years ago

Everything looks great except I seem to be getting the same ... 'filter' of undefined when doing a build 😕

Off the top of my head not sure what the issue might be.

i'm kinda glad that you saw it too, means it's not just my computer having the issue somehow. Since the last 3 builds of this branch remotely have passed i'm going to shelf looking further into the issue to handle it separately.