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

Optimize Flash component #724

Merged
merged 6 commits into from
Jun 22, 2020
Merged

Optimize Flash component #724

merged 6 commits into from
Jun 22, 2020

Conversation

Terris
Copy link
Contributor

@Terris Terris commented Jun 19, 2020

Now that Flash is fully integrated I'm already seeing some ways it could be optimized (of course). This PR serves to make those optimizations.

  • Add appropriate dependencies to useEffect.
  • timeout functionality should only result in visual changes and not cause an unnecessary render.
  • visual timeout should not be a jarring flash - now it's a slide up.

@thedavidprice
Copy link
Contributor

@Terris FYI, we just flipped the default from 'master' --> 'main'. #718

Wanted to make sure you knew!

@Terris
Copy link
Contributor Author

Terris commented Jun 19, 2020

@Terris FYI, we just flipped the default from 'master' --> 'main'. #718

Wanted to make sure you knew!

Aha, I missed that. Thanks for the heads up.

@peterp peterp self-requested a review June 19, 2020 15:56
@Terris Terris marked this pull request as ready for review June 19, 2020 17:14
@peterp
Copy link
Contributor

peterp commented Jun 19, 2020

Hey @Terris, I'm busy running through our e2e tests for the tutorial and I'm getting an error message that "viewed" is undefined.

(This occurs when I click on the delete button after creating a post.)

Is this something that you know about, and is covered in this PR, or would you like me to open a separate issue for this?

Screen Shot 2020-06-19 at 22 32 39
Screen Shot 2020-06-19 at 22 33 01

@Terris
Copy link
Contributor Author

Terris commented Jun 19, 2020

@peterp Dang, I'm not able to reproduce the error you are seeing. Is there any further log info that might prove useful?

@peterp
Copy link
Contributor

peterp commented Jun 20, 2020

@Terris Ok, no problem, I'll try to reproducce it outside of Cypress and I'll open a ticket.

@peterp peterp added this to the next release milestone Jun 22, 2020
@peterp peterp merged commit c47de03 into redwoodjs:main Jun 22, 2020
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

3 participants