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

Don't fetch manifest when all threads is set #464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bmulholland
Copy link

Code changes

We include a fully stubbed-out/mocked-out instance of the mailbox component in our Storybook code. We then use Chromatic to take snapshots of it. This way, when we update the component, we'll get a visual comparison of any UI changes. It will help us upgrade our components quickly and easily, while still knowing if our UI will change somehow.

However, despite us supplying external data with all_threads, the component still makes an HTTP request to Nylas, requiring both a component ID and Nylas Access Token. We don't want to expose a secret token to access a mailbox to our CI and Storybook; to date, our frontend has zero secrets that cannot be exposed to the frontend. We'd like to keep it that way.

This change simply skips the manifest loading when all_threads is set. None of the manifest data would use used in this situation, anyway.

Readiness checklist

  • Added changes to component CHANGELOG.md
  • New property added? make sure to update component/src/properties.json
  • Cypress tests passing?
  • New cypress tests added?
  • Included before/after screenshots, if the change is visual

License

I confirm that this contribution is made under the terms of the MIT license and that I have the authority necessary to make this contribution on behalf of its copyright owner.

@vercel
Copy link

vercel bot commented Oct 14, 2022

@bmulholland is attempting to deploy a commit to the Nylas Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Oct 14, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
components ❌ Failed (Inspect) Oct 18, 2022 at 2:42PM (UTC)

@AaronDDM AaronDDM requested review from yifanplanet, a team and pooja169usp and removed request for a team October 14, 2022 17:41
@yifanplanet yifanplanet removed the request for review from pooja169usp January 9, 2023 19:31
@yifanplanet yifanplanet removed their request for review March 17, 2023 14:57
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