Skip to content

Conversation

@k-fish
Copy link
Member

@k-fish k-fish commented Oct 25, 2021

Summary

This fills out the frontend-other tab as well as fixing a couple issues with local storage between tabs (using a provider to add additional context about the current view to the key).

This fills out the frontend-other tab as well as fixing a couple issues with local storage between tabs (using a provider to add additional context about the current view to the key, the name isn't great though).
@k-fish k-fish requested a review from a team October 25, 2021 15:33
name: 'CurrentPerformanceViewContext',
});

export {CurrentPerformanceViewProvider};
Copy link
Member Author

Choose a reason for hiding this comment

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

The current performance view context is a bit of a weird name, suggestions welcome.

Copy link
Member

Choose a reason for hiding this comment

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

How about just PerformanceDisplayProvider?

@github-actions
Copy link
Contributor

size-limit report

Path Base Size (84e73fe) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 52.74 KB 52.74 KB +0.01% 🔺
src/sentry/static/sentry/dist/entrypoints/sentry.css 70.89 KB 70.89 KB 0%

name: 'CurrentPerformanceViewContext',
});

export {CurrentPerformanceViewProvider};
Copy link
Member

Choose a reason for hiding this comment

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

How about just PerformanceDisplayProvider?

Comment on lines 51 to 55
const key = getContainerKey(index, performanceType, height);
const localObject = JSON.parse(
localStorage.getItem(getContainerLocalStorageObjectKey) || '{}'
);
const value = localObject?.[key];
Copy link
Member

Choose a reason for hiding this comment

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

Should this be refactored into some utility function to handle the parsing instead of directly using the localStorage API now that you're storing an object?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's a good call.

@k-fish k-fish merged commit eefbce4 into master Oct 26, 2021
@k-fish k-fish deleted the feat/perf-landing-add-frontend-other branch October 26, 2021 17:25
@github-actions github-actions bot locked and limited conversation to collaborators Nov 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants