Add network/storage mappings table component #23

ibolton336 · 2020-09-02T15:21:31Z

Adds necessary logic to populate the shared mappings table with network or storage mappings.
Adds Add mapping button to the top of the shared table. This button opens the shared mappings modal
Uses mock data in local storage for testing. There are a few throwaway functions added to /mocks/helpers for this purpose. We can use these until we are ready for redux implementation.

Closes #31

konveyor-preview-bot · 2020-09-02T15:23:29Z

🚀 Deployed Preview: http://konveyor-virt-ui-pr-23-preview.surge.sh ✨

mturley

Looking good! Just a couple things.

mturley · 2020-09-08T20:18:45Z

src/app/Mappings/mocks/helpers.ts

@@ -0,0 +1,32 @@
+import { MappingType } from '../types';
+
+export const updateMockStorage = (params: any) => {


We might want to add a comment here that this is temporary, so we don't invest too much time making it better / spreading it.

mturley · 2020-09-08T20:20:07Z

src/app/Mappings/Network/NetworkMappingsPage.tsx


 const NetworkMappingsPage: React.FunctionComponent = () => {
+  const [networkMappings, setNetworkMappings] = React.useState([]);


Let's throw a comment here too saying this is temporary, so we don't end up duplicating redux state here in react land

mturley · 2020-09-08T20:22:28Z

src/app/Mappings/Network/NetworkMappingsPage.tsx

+
+  //TODO replace with real state from redux
+  const mockMapObj = localStorage.getItem('networkMappingsObject');
+  React.useEffect(() => {


So I'm not sure I get the useEffect code here, why does it have mockMapObj in its dependency array? That won't change here I don't think, so the useEffect will only be triggered once. I think we can get rid of the mockMapObj and have [] as the deps array for the effect?

The mockMapObj is required in the dependency array because when I added an item to the local storage, there was no rerender triggered. This will for sure go away when we move to real redux state but it is needed temporarily for the mock localstorage code to work for now.

got it. ok.

mturley · 2020-09-08T20:23:20Z

src/app/Mappings/Storage/StorageMappingsPage.tsx


 const StorageMappingsPage: React.FunctionComponent = () => {
+  const [storageMappings, setStorageMappings] = React.useState([]);


Same comments here: comment about temporary state, remove mockMapObj (I think?)

mturley · 2020-09-08T20:24:12Z

src/app/Mappings/components/AddEditMappingModal.tsx

+          variant="primary"
+          onClick={() => {
+            //TODO: Replace with a real redux call for adding a mapping
+            updateMockStorage({ mappingName, sourceProvider, targetProvider, mappingType });


We'll need to change this after we merge #25, we can generate the whole mapping object with a helper function I wrote there

mturley · 2020-09-08T20:31:22Z

src/app/Mappings/components/MappingsTable.tsx

 }: IMappingsTableProps) => {
+  const getSortValues = (mapping: ICommonMapping) => {


I think we should use Mapping instead of ICommonMapping here. Mapping is a union of the two specific mapping interfaces, so a mapping with network-specific or storage-specific properties is a valid Mapping but not a valid ICommonMapping (which is just what those two interfaces extend).

mturley · 2020-09-08T20:31:54Z

src/app/Mappings/components/MappingsTable.tsx

+  const {
+    selectedItems: expandedMappings,
+    toggleItemSelected: toggleMappingExpanded,
+  } = useSelectionState<ICommonMapping>(sortedItems);


Same here, Mapping instead of ICommonMapping. Maybe I shouldn't have even exported it.

mturley · 2020-09-08T20:37:16Z

src/app/Mappings/components/MappingsTable.tsx

+  const rows: IRow[] = [];
+  currentPageItems.forEach((mapping: ICommonMapping) => {
+    const { name, sourceProvider, targetProvider, items } = mapping;
+    const isExpanded = expandedMappings.includes(mapping);


This will be fine for now, but I'm actually realizing that if the mappings objects in redux are updated, this referential equality will fail to match and we'll lose our expanded state. We might need to use the isItemSelected function exported by the useSelectionState hook, and pass an isEqual function into it that uses ids or something. I do that in my examples here: https://konveyor.github.io/lib-ui/?path=/docs/hooks-useselectionstate--checkboxes

I probably need to make the same change elsewhere, that'll become apparent as we start to use redux for these

Added a comment for now. We should open a PR to pull in the useSelection state from lib-ui also. PR opened #32

mturley · 2020-09-08T20:38:52Z

src/app/Mappings/components/MappingsTable.tsx

+    {
+      title: <>{mappingType === MappingType.Network ? 'Network mappings' : 'Storage mappings'}</>,
+      transforms: [sortable],
+      cellTransforms: [compoundExpand],


This is another place where we need to remove compoundExpand, this and the import.

mturley · 2020-09-08T20:39:41Z

src/app/Mappings/types.ts

@@ -28,7 +28,12 @@ export interface IStorageMappingItem {
 export interface ICommonMapping {
  type: MappingType;
  name: string;
-  provider: {
+  sourceProvider: {


This might conflict with #25, I think I made the same change there.

* update TS support for network/storage mappings * add comment to remove throwaway function

mturley

LGTM 👍

ibolton336 requested a review from a team September 2, 2020 15:21

mturley reviewed Sep 8, 2020

View reviewed changes

ibolton336 added 4 commits September 8, 2020 17:23

wip

6df66e9

get mock data mappings table working

93ce033

move mock function to its own file

0e7e2c2

Wire up storage mapping mock

1860609

ibolton336 force-pushed the network-mappings-table branch from 2b06e3c to 1860609 Compare September 8, 2020 21:24

ibolton336 and others added 4 commits September 8, 2020 17:31

cleanup merge

18ba92e

update PR

4c3d913

cleanup

4dbfcb8

Origin network mappings table (#33)

6b8737f

* update TS support for network/storage mappings * add comment to remove throwaway function

mturley approved these changes Sep 9, 2020

View reviewed changes

mturley merged commit a8f1e6f into master Sep 9, 2020

mturley deleted the network-mappings-table branch September 9, 2020 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add network/storage mappings table component #23

Add network/storage mappings table component #23

ibolton336 commented Sep 2, 2020 •

edited

Loading

konveyor-preview-bot commented Sep 2, 2020

mturley left a comment

mturley Sep 8, 2020

mturley Sep 8, 2020

mturley Sep 8, 2020

ibolton336 Sep 8, 2020

mturley Sep 9, 2020

mturley Sep 8, 2020

mturley Sep 8, 2020

mturley Sep 8, 2020

mturley Sep 8, 2020

mturley Sep 8, 2020

ibolton336 Sep 8, 2020 •

edited

Loading

mturley Sep 8, 2020

mturley Sep 8, 2020

mturley left a comment

		@@ -0,0 +1,32 @@
		import { MappingType } from '../types';

		export const updateMockStorage = (params: any) => {


		const NetworkMappingsPage: React.FunctionComponent = () => {
		const [networkMappings, setNetworkMappings] = React.useState([]);


		const StorageMappingsPage: React.FunctionComponent = () => {
		const [storageMappings, setStorageMappings] = React.useState([]);

		}: IMappingsTableProps) => {
		const getSortValues = (mapping: ICommonMapping) => {

Add network/storage mappings table component #23

Add network/storage mappings table component #23

Conversation

ibolton336 commented Sep 2, 2020 • edited Loading

konveyor-preview-bot commented Sep 2, 2020

mturley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibolton336 Sep 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mturley left a comment

Choose a reason for hiding this comment

ibolton336 commented Sep 2, 2020 •

edited

Loading

ibolton336 Sep 8, 2020 •

edited

Loading