[KED-2911] Implement the comparison-view journey #619
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n to display metadata
… and PrimaryToolBar components
studioswong
reviewed
Nov 9, 2021
…into feature/implement-comparison-view-journey#ked-2911
…ithub.com/quantumblacklabs/kedro-viz into feature/implement-comparison-view-journey#ked-2911
studioswong
suggested changes
Nov 9, 2021
There was a problem hiding this comment.
Thanks for this PR! the styling and UI setup looks good.
Some weird things I noticed while playing around with this branch:
- under single view, the metadata on the experiment details panel didn't seem to update on selecting different experiments - is this due to the setup of the dev server hosted on heroku? ( see below screenshot)
- This also applies to the comparison view as well - the experiments on the details panel doesn't seem to align with the selection on the sidebar. This particularly seem to be the case when de-selecting a run on the sidebar. ( see below)
Some further suggestions / comments on the UI flow:
- On first load of the page, the latest experiment ( last item in the runsList array) should be selected and loaded on default.
- Under the single view, clicking an already selected experiment on the sidebar should not render any action instead of being de-selected ( currently it will deselect that experiment with details component rendering nothing).
src/components/experiment-tracking/run-metadata/run-metadata.scss
Outdated
Show resolved
Hide resolved
Comment on lines
+42
to
+49
| {(active || enableComparisonView) && ( | ||
| <CheckIcon | ||
| className={classnames('runs-list-card__checked', { | ||
| 'runs-list-card__checked--active': active, | ||
| 'runs-list-card__checked--comparing': enableComparisonView, | ||
| })} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
Would be good to add a test to test the addition of the checkIcon for both active and enableComparisonView states
There was a problem hiding this comment.
I'm not able to successfully test the active state. Maybe you can help me with that? I'm still learning the ins and outs of Jest.
… between query results
studioswong
approved these changes
Nov 10, 2021
studioswong
deleted the
feature/implement-comparison-view-journey#ked-2911
branch
studioswong
added a commit
that referenced
this pull request
Nov 23, 2021
* [KED-2872] Create global toolbar UI (#576) * adds in the global toolbar component; moves settings and theme buttons to global toolbar * adds new logo icons for kedro, pipeline, and experiments and adds them to the global sidebar * adds new test file for global-toolbar; updates primary-toolbar tests * updates icon styles and order; removes unnecessary tests and initial state values * removes two unneeded App propTypes * KED-2875: Set up routing infrastructure and component placeholders for experiment tracking page (#582) * setup of experiment router route * setup flowchart Wrapper and refactored old wrapper component * refadctored sidebar and added experiment route * Set up runlist sidebar, runsDetails and basic routing * delete unneeded flowchart wrapper test * modify route to runsList and refactor to use queryparams for routing * receive unneccessary change to wrapper scss file * added comment for experiment details wrapper * added memoryRouter to test and delete visible prop * set up useRunIdsFromUrl hook and refactor to use fragments * change from flowchart to flowChart * [KED-2882] Create experiment tracking sidebar UI (#587) * create sidebar runs card; add some simple tests; add necessary colors * add a collapsable container and necessary tests; small renaming and cleanup * add comments * update default accordion header value; remove unneeded margins * capitalization * update light bg color on run card; use && instead of ternary; add border-bottom to last run card; remove some placeholder content * update runs-list-card tests * [KED-2849] & [KED-2851] Set up GraphQL mock server (#604) * set up graphql schema * set up schema and mocks for dev server, along with apollo mock provider and client * update to runsList component to take in mock data * updates to mocks * further updates to mock setup * update to comments in mocks * updates to data type of run-list-card * added fetch library as polyfill for lib-test import * added new props to runs-list * set up schema.graphql and updates to schema * added fetch back in to enable testJS import test to pass on CI * added fetch and createHttpLink * [KED-2883] create experiment details UI (#605) * create a RunMetadata component and style accordingly * container and single-run styling * add the show more button to the notes area * update button font * starting on the metric details work * expand accordion component to include another layout and different size titles * ui driven by common mock-data file * shared accordions positioned, styled, and collapsing correctly * better mock data * scroll metadata and metric together * move RunMetrics code into own component; style tweaks * solid refactor of the RunMetrics component * some renamning; add some comments * refactor dataset accordions section after new data shape agreed upon * fix failing RunListCard test * add a show less button; add more accordion tests * add tests for RunMetadata and RunDataset components * add headingClassName to RunsList accordion * element alignment * update trackingData shape and refactor RunDataset code accordingly * [KED-2911] Implement the comparison-view journey (#619) * update mocks to align with schema from miro; begin runs list selection to display metadata * toggle comparison view working and updating RunsList; reusing Sidebar and PrimaryToolBar components * single run selection/deselection working * comparison run selection/deselection working * style adjustments; test with dummy graphql server * fix failing tests * create a Switch component and wire it up; remove the dummy icon in the PrimaryToolbar * connect to deployed dummy graphql endpoint * empty commit to see if pytest works * pr fixes; create a apollo hooks wrapper function to better transition between query results * prevent single run deselection in comparison view mode * more robust test for RunsListCard component * add the currently designed empty runs screen Co-authored-by: Rashida Kanchwala <rashida.kanchwala@quantumblack.com> * update http link to use actal graphql endpoint * update schema and queries with new showDiff variable (#624) * update schema and queries with new showDiff variable * updated schema and queries per PR comments * updates to queries and tracking details component * added queries back in * added changes to component level for graphql data * further modify naming of value * KED-2944: integrate the real API with experiment tracking frontend (#630) * update http link to use actal graphql endpoint * updates to queries and tracking details component * added queries back in * added changes to component level for graphql data * [KED-2954] Format experiment tracking timestamp (#625) * create a toHumanReadableTime date util and use in experiment tracking * fix failing tests * graphql endpoint back to local viz url * update formatting for timestamp in toHumanReadableTime function * simplify toHumanReadableTime logic * update lock file Co-authored-by: Tynan DeBold <thdebold@gmail.com> Co-authored-by: Rashida Kanchwala <rashida.kanchwala@quantumblack.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements the single and comparison view journey, connecting and wiring up all the necessary pieces of UI.
Development notes
<Sidebar />component for both experiment tracking and flowchart views.<ExperimentWrapper />down to children) though it still feels very manageable to me. Perhaps we can better leverage the Apollo cache as we go forward.<RunsListCard />and the metadata and tracking data render. I'm not sure why this is happening. Would appreciate another eye on it.<Switch />component, which is used above the<RunsList />component to toggle between the single and comparison views.QA notes
Best to checkout the branch and run it locally to test everything.
NOTE: there's currently a failing Python test in the build pipeline. @AntonyMilneQB is looking into it.
Checklist
RELEASE.mdfileLegal notice
I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":
I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorised to submit this contribution on behalf of the original creator(s) or their licensees.
I certify that the use of this contribution as authorised by the Apache 2.0 license does not violate the intellectual property rights of anyone else.