-
Notifications
You must be signed in to change notification settings - Fork 969
feat: Use type metadata description get/update apis #1876
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,22 +65,11 @@ exports[`eslint`] = { | |
[118, 19, 20, "Must use destructuring state assignment", "3067028466"], | ||
[126, 10, 137, "Visible, non-interactive elements with click handlers must have at least one keyboard listener.", "3965651236"] | ||
], | ||
"js/components/EditableText/index.tsx:2168483193": [ | ||
"js/components/EditableText/index.tsx:1961540365": [ | ||
[51, 2, 21, "textAreaRef should be placed after componentDidUpdate", "3072052035"], | ||
[72, 6, 13, "Do not use setState in componentDidUpdate", "57229240"], | ||
[80, 10, 25, "Must use destructuring props assignment", "793704523"], | ||
[81, 8, 25, "Must use destructuring props assignment", "793704523"], | ||
[83, 32, 16, "Must use destructuring state assignment", "3998965439"], | ||
[83, 53, 21, "Must use destructuring state assignment", "1159122654"], | ||
[85, 6, 13, "Do not use setState in componentDidUpdate", "57229240"], | ||
[90, 4, 22, "Must use destructuring props assignment", "2225424112"], | ||
[94, 4, 22, "Must use destructuring props assignment", "2225424112"], | ||
[98, 27, 23, "Must use destructuring props assignment", "2982501243"], | ||
[101, 23, 23, "Must use destructuring props assignment", "2982501243"], | ||
[109, 6, 22, "Must use destructuring props assignment", "2225424112"], | ||
[116, 4, 24, "Must use destructuring props assignment", "3049746099"], | ||
[134, 19, 20, "Script URL is a form of eval.", "3373049033"], | ||
[146, 8, 207, "A control must be associated with a text label.", "2914986446"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks a lot! |
||
[78, 6, 13, "Do not use setState in componentDidUpdate", "57229240"], | ||
[95, 6, 13, "Do not use setState in componentDidUpdate", "57229240"], | ||
[148, 19, 20, "Script URL is a form of eval.", "3373049033"] | ||
], | ||
"js/components/EntityCard/EntityCardSection/index.tsx:474926744": [ | ||
[25, 2, 55, "editButton should be placed after constructor", "2479426463"], | ||
|
@@ -444,9 +433,8 @@ exports[`eslint`] = { | |
[19, 2, 8, "Assignment to function parameter \'resource\'.", "2131237679"], | ||
[20, 2, 248, "Expected a default case.", "1034339850"] | ||
], | ||
"js/ducks/tableMetadata/api/v0.ts:4194065289": [ | ||
[80, 8, 23, "Use object destructuring.", "1142306891"], | ||
[139, 23, -4311, "Expected to return a value at the end of arrow function.", "5381"] | ||
"js/ducks/tableMetadata/api/v0.ts:1365865963": [ | ||
[140, 23, -4339, "Expected to return a value at the end of arrow function.", "5381"] | ||
], | ||
"js/ducks/tableMetadata/owners/index.spec.ts:655040122": [ | ||
[15, 0, 91, "\`../reducer\` import should occur before import of \`./reducer\`", "2216296793"], | ||
|
@@ -455,8 +443,8 @@ exports[`eslint`] = { | |
"js/ducks/tableMetadata/owners/sagas.ts:3725515638": [ | ||
[7, 0, 69, "\`../types\` import should occur before import of \`./reducer\`", "3326352266"] | ||
], | ||
"js/ducks/tableMetadata/reducer.ts:1012064960": [ | ||
[416, 6, 93, "Unexpected lexical declaration in case block.", "4098864482"] | ||
"js/ducks/tableMetadata/reducer.ts:3842494077": [ | ||
[480, 6, 93, "Unexpected lexical declaration in case block.", "4098864482"] | ||
], | ||
"js/ducks/tags/api/v0.ts:2781466514": [ | ||
[21, 16, -605, "Expected to return a value at the end of function \'getResourceTags\'.", "5381"] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -115,3 +115,29 @@ export function getColumnCount(columns: TableColumn[]) { | |
columns.length | ||
); | ||
} | ||
|
||
/** | ||
* Given a type metadata key, returns the associated type metadata object | ||
*/ | ||
export function getTypeMetadataFromKey( | ||
tmKey: string, | ||
tableData: TableMetadata | ||
) { | ||
const tmNamePath = tmKey.replace(tableData.key + '/', ''); | ||
|
||
const [ | ||
columnName, | ||
typeConstant, // eslint-disable-line @typescript-eslint/no-unused-vars | ||
topLevelTmName, // eslint-disable-line @typescript-eslint/no-unused-vars | ||
Comment on lines
+130
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can do:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case I was getting the error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, makes sense to complain with two of those. |
||
...tmNames | ||
] = tmNamePath.split('/'); | ||
|
||
const column = tableData.columns.find((column) => column.name === columnName); | ||
|
||
let typeMetadata = column?.type_metadata; | ||
tmNames.forEach((name) => { | ||
typeMetadata = typeMetadata?.children?.find((child) => child.name === name); | ||
}); | ||
kristenarmes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
return typeMetadata; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so right now editability is only controlled down to the parent level? Lets say we want to set a nested column to be have an editable description we can't then set one of it's sub column's to not have an editable description right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true as of now, I followed how the columns are handled as well, since it appears that as of now it only has table level configurations to determine whether columns are editable or not. I think this could be adjusted in the future pretty easily if that were to change