Skip to content
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

Navigation Component: Empty or undefined verticalKey has same behavio… #578

Merged
merged 2 commits into from
Mar 23, 2020

Conversation

alexisgrow
Copy link
Contributor

…r as null

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.

…r as null

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
@@ -99,7 +99,7 @@ export class Tab {

// For tabs without config ids, map their URL to the configID
// to avoid duplication of renders
if (tab.verticalKey === null && tabs[tab.verticalKey] === undefined) {
if (!tab.verticalKey && !tabs[tab.verticalKey]) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know it was like this in the old code, but do we need the && !tabs[tab.verticalKey] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good point, is this the logic we should have:
if (!tab.verticalKey && !tabs[tab.url])?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes that looks good to me!

Copy link
Contributor

@oshi97 oshi97 left a comment

Choose a reason for hiding this comment

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

lgtm!

@alexisgrow alexisgrow merged commit 7b69165 into v0.14.0 Mar 23, 2020
@alexisgrow alexisgrow deleted the navigation-null-checks branch March 23, 2020 17:11
oshi97 pushed a commit that referenced this pull request Mar 31, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 1, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 1, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 1, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 1, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 1, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 2, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 6, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 6, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
oshi97 pushed a commit that referenced this pull request Apr 6, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
alexisgrow added a commit that referenced this pull request Apr 7, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
alexisgrow added a commit that referenced this pull request Apr 22, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
tmeyer2115 pushed a commit that referenced this pull request Apr 24, 2020
#578)

If an implementer passed in a verticalKey that was empty or undefined, its
entry would be ignored in the navigation component. Changing the null check
so any falsy verticalKey is treated the same.

TEST=manual

Test on local answers instance that universal verticalPages config is
used when omitting vertical key.
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.

None yet

2 participants