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

fix: Stories rendering issues #7222

Merged
merged 1 commit into from
Jan 14, 2022
Merged

Conversation

parasharrajat
Copy link
Member

@parasharrajat parasharrajat commented Jan 14, 2022

Details

Fixed Issues

$ #7187

Tests | QA Steps

  1. Go to staging.new.expensify.com/docs/index.html
    and verify that you can see the storybook docs with the themes matching the screenshots below.
  2. Canvas / Header/ Default
  3. Canvas / ButtonWithDropdwn/ Fefault
  4. Canvas/ MenuItem/ Default
  5. Repeat step 2-4 for Docs tab
  6. Go to HeaderWithCloseButton
  • Verify that no errors appear in the JS console

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

image508579.png)

Mobile Web

Desktop

iOS

Android

@parasharrajat parasharrajat requested a review from a team as a code owner January 14, 2022 13:33
@MelvinBot MelvinBot requested review from mountiny and removed request for a team January 14, 2022 13:33
@mountiny
Copy link
Contributor

Is this ready for a review?

@parasharrajat
Copy link
Member Author

Yeah, Stories only work on the Web. So only a screenshot of the Web is attached.

@mountiny
Copy link
Contributor

Yeah yeah, I see you did not run npm install to get the new design and Expensify logo in the Storybook :)

@mountiny
Copy link
Contributor

@parasharrajat Just to confirm, I keep seeing these console errors locally
image

When I seacherd for these previously, these avraibles are not defined in our codebase so I assume this must be a problem inside Storybook. What do you think?

@parasharrajat
Copy link
Member Author

Let me run thenpm install first.

@parasharrajat
Copy link
Member Author

I checked the storybook local server and I don't see any error.
image

Please tell me if you are using something else.

@mountiny
Copy link
Contributor

mountiny commented Jan 14, 2022

@parasharrajat nono I am using the same. That is odd.

Whenever I switch between the components, I get those 3 errors.

@parasharrajat
Copy link
Member Author

An interesting fact is I can't find these strings in our codebase. 😃

@mountiny
Copy link
Contributor

Yeah, I said that before too which makes me think it must be from Storybook. Well, let's try this on staging then 😄 Thank you for working on this!

@mountiny mountiny merged commit de8d439 into Expensify:main Jan 14, 2022
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by @mountiny in version: 1.1.29-6 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 failure ❌
🕸 web 🕸 success ✅

@kavimuru
Copy link

@parasharrajat PR is failing. Still we see console errors. Able to reproduce this bug #7187
chrome_DnwJhUVZLL
7

@parasharrajat
Copy link
Member Author

These are cors and security errors. It does not break the Storybook in any way. You can see that headers are rendered correctly on the Canvas.
It is about the deployment and configuration of the Storybook so it should be handled on the backend. To me, it does not matter much but I will leave this to @mountiny to decide.
image

@mountiny
Copy link
Contributor

Yes, that is true. These errors are cors related and not sure if there is anything simple we can do about this. These errors are not reproducible in the local dev environment.

@kavimuru I think we will have to accept these particular errors will be around for Storybook for a while. Would you be able to reflect it in your QA steps, please? They always refer Content Security Policy or CSP.

Can you also label this as passing the QA? It has resolved the other issues and Rajat can't do much about this particular type of errors.

@OSBotify
Copy link
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.30-3 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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.

4 participants