IGVF-1305 Redesign home page #402
Conversation
components/navigation.js
Outdated
|
|
||
| return ( | ||
| <li> | ||
| <a |
There was a problem hiding this comment.
Since it is an external site, should it open in a new tab instead of the original tab?
There was a problem hiding this comment.
I usually think so, but the igvf.org site links to our page without opening a new tab, so I felt maybe it makes sense to return to that page without opening a new tab.
There was a problem hiding this comment.
Oh wait, you’re talking about the catalog. Yeah, I thought about that, but I really wasn’t sure.
components/chart-file-set.js
Outdated
|
|
||
| // The legend should only appear while the user is logged in. | ||
| const { isAuthenticated } = useAuth0(); | ||
| const legendProps = isAuthenticated |
There was a problem hiding this comment.
Why legend is hidden if not logged in? I don't feel that this is sensitive data.
There was a problem hiding this comment.
The legend shows the color differences between released and non-released file sets in the chart. But while logged out you only see released file sets, so Meenakshi felt it didn’t make sense to show this legend when you’d only ever see one color in the chart.
|
Thanks! On Safari, that’s a known issue when you have “Prevent cross-site tracking” enabled in Privacy settings. I have a solution to this, but I can only test it in a demo on weekends. That’s why I ran into an issue of my demos getting automatically deleted as soon as they came up. |
forresttanaka
force-pushed
the
IGVF-1305-home-page
branch
4 times, most recently
from
7bf0d1b
to
7fb27d4
Compare
* Display file-set data in an initial nivo chart. * Change colors and height of chart. * Make custom Y-axis labels. Move the legend to the bottom. * Refactor the charting code a bit. Move legend to the left side to make space for a month selector. Remove the chart hover tooltips. Add initial titles and statistics boxes to the home-page header. * Make the width of the Y-axis label column easily changeable. Fix some of the comments. * Update the data-provider request to include the creation timestamp. * Implement the month selector that filters the list of measurement sets displayed. * Format each bar’s aria label. * Don’t show fractional scale values. * Make dark mode work. * Add home-page title. * Fix the tick marks so that large values don’t cause the ticks to mush together. * Fix the bottom-axis tick numbers so they always include 0 and the maximum value. Abbreviate the bottom-axis values. * Finish the statistics boxes. * Hide chart on narrow pages. * Increase the Cypress timeout to 60 seconds from 30 seconds. Add a delay after clearing the text area before typing into it in the Cypress add.cy.js file to avoid having the new text append the existing text. * Change Cypress timeout from 60s to 65s to see if it finally triggers on CircleCI. * Try going back to the original 30s timeout, then wait 10s after logging in before calling cy.visit, instead of just one second. * Maybe cy.visit() breaks the Cypress test. Instead, navigate to the Schemas page for the * Try not retrieving data for home page to see if this helps Cypress pass. Set Cypress timeout back to 60 seconds. * Add a command timeout of 60 seconds specifically for add.cy.js to see if that helps. * Return getServerSideProps() code to retrieve home-page data. Change add.cy.js defaultCommandTimeout to 90 seconds because the last Cypress/CircleCI run had the editor appear exactly the moment Cypress timed out the test. Wait two seconds instead of one after logging in, as the menu commands cut in slightly early. * Another CircleCI/Cypress failure. Now try not rendering the graph. If that works, I don’t know where to go from there. * Fix build error in last commit. * Make home-page chart conditional, only rendering if Cypress doesn’t run. * Revert many changes to the Cypress tests, and remove the container query from `<App>` as it seems like that caused the Cypress failures for some reason. * Add IGVF Consortium link to the home-page title area. * Update the statistics boxes for dark mode. Change the title of the Measurement Set statistics box. Change the Lab statistics box to show total Sample objects. * Have legend only appear while the user is logged in. * Reorganize the functions within chart-file-set.js. Correct some comments. Add a Cypress test for the home page. * Try making the Cypress browser size larger to make the chart appear. * Do a little code reorganization. * Use the floating ui package to manage the month-selector popup instead of handling everything here. That lets us take advantage of floating ui’s automatic positioning. * Move non-React-component functions to a new file in lib/ so we can cover them with Jest tests. * Have home-page Cypress test work with default igvfd data, not including the MeasurementSet additions I made. * Make the month selector accessible. * Add Jest tests for the home.ts library. Convert the home.js library to Typescript. * Correct a comment. * Remove Cypress tests for the home page chart bars because they’re all for the same date. * Change the icon colors. * Correct some comments. Add some attributes for accessibility. * Convert statistics panels to linked buttons. * Colorize the three statistics buttons. Change month-selector button into a primary style. * Adjust color of the Samples statistics button. * Add catalog links to the main navigation. * Change the coloring on the statistics buttons. * Delete now-unused home-page images. * Fix a merge marker I missed last time. * Add the cumulative release chart. Remove Catalog section. * Format the month labels. * Abbreviate numbers used in the Y axis labels. * Make the Y axis legend work with dark mode. * Replace the month-selection dropdown code with the new dropdown module. Move the text-color Tailwind CSS out of an `<App>` div and into the body tag so that root-level portals get the automatic text color. * Clean up chart title usage. Change the status chart legend depending on if the user has logged in or not, instead of hiding the legend if the user hasn’t logged in. * Have home.test.js pass. * Add Jest test for converting file-set data to release data. Get full coverage for other Jest tests. * Fix home-page Cypress tests. * Add Catalog button to the home-page title. * Make sure the home page works even with no MeasurementSet release data. * Correct comments. Remove the now-unused NavigationAnchorItem component.
forresttanaka
force-pushed
the
IGVF-1305-home-page
branch
from
7fb27d4
to
c48ec58
Compare
|
Up for code review for the second time with an entire new chart added. You’ll see I moved the Tailwind CSS classes for light and dark mode text color from inside the |
|
@mingjiecn Yeah, I know about that, but this goes away once we get more than a handful of released data sets, so given the difficulty of adding a custom Y-tick generator (I have an X-tick generator for the other chart because that issue could linger for a while), I felt it was OK not to fix this. Am I being too cavalier? |
|
Sounds reasonable. I will go ahead to approve it. |
|
@mingjiecn Thanks! I’m completely wrong though. After you said that, I looked into it more, and for this chart’s Y axis, the fix is way simpler than the more complicated X axis of the bar chart. In fact, it’s just the addition of one more integer attribute in the Nivo config. So I’ll make that change. Since the fix is so simple, I’ll just make it and go to QA. Thanks for making me look at that again! I had figured it would be as complicated as the X-axis case for the bar chart, so it wasn’t worth worrying about. |
Reduce the line-chart Y-axis ticks for small amounts.
Updates from Idan’s feedback.
When displaying the chart while using a dev build, you see a console warning, “Failed prop type: Converting circular structure to JSON.” As described in this thread, this seems like a bug in PropTypes. PropTypes don’t run in production builds, so that causes no problems there.