Skip to content

Conversation

@JamesHollyer
Copy link
Contributor

Motivation for features / changes

In #6346 we updated the ColumnHeader interface. However, objects using this interface are stored in localStorage. This checks localStorage to ensure it has the new interface. If it does not then it ignores those values.

Technical description of changes

I am simply checking if the first value has the name property to ensure the values are valid. I could do a more thorough check but it seems unnecessary.

Detailed steps to verify changes work correctly (as executed by you)

It worked for me when running locally.

@JamesHollyer JamesHollyer requested a review from rileyajones May 3, 2023 16:24
Comment on lines 211 to 216
if (
backendSettings.hasOwnProperty('singleSelectionHeaders') &&
typeof backendSettings.singleSelectionHeaders === 'object'
Array.isArray(backendSettings.singleSelectionHeaders) &&
// If the settings stored in the backend are invalid, reset back to default.
backendSettings.singleSelectionHeaders[0].name !== undefined
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this initial condition anymore. FYI Array.isArray(undefined) returns false.

Suggested change
if (
backendSettings.hasOwnProperty('singleSelectionHeaders') &&
typeof backendSettings.singleSelectionHeaders === 'object'
Array.isArray(backendSettings.singleSelectionHeaders) &&
// If the settings stored in the backend are invalid, reset back to default.
backendSettings.singleSelectionHeaders[0].name !== undefined
) {
// If the settings stored in the backend are invalid, reset back to default.
if (Array.isArray(backendSettings).singleSelectionHeaders && backendSettings.singleSelectionHeaders[0].name !== undefined) {

Comment on lines 220 to 225
if (
backendSettings.hasOwnProperty('rangeSelectionHeaders') &&
typeof backendSettings.rangeSelectionHeaders === 'object'
Array.isArray(backendSettings.rangeSelectionHeaders) &&
// If the settings stored in the backend are invalid, reset back to default.
backendSettings.rangeSelectionHeaders[0].name !== undefined
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here

Suggested change
if (
backendSettings.hasOwnProperty('rangeSelectionHeaders') &&
typeof backendSettings.rangeSelectionHeaders === 'object'
Array.isArray(backendSettings.rangeSelectionHeaders) &&
// If the settings stored in the backend are invalid, reset back to default.
backendSettings.rangeSelectionHeaders[0].name !== undefined
) {
// If the settings stored in the backend are invalid, reset back to default.
if (
Array.isArray(backendSettings.rangeSelectionHeaders) &&
backendSettings.rangeSelectionHeaders[0].name !== undefined
) {

@JamesHollyer JamesHollyer merged commit 5ea918e into tensorflow:master May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants