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

Added subfolder to initial theme state to eliminate render error #652

Merged
merged 1 commit into from
May 23, 2024

Conversation

pbking
Copy link
Contributor

@pbking pbking commented May 23, 2024

To test:

Load the Metadata Modal window with the browser console open.

Note that no errors are shown. (There should be no other changes.)

image
Copy link
Member

@vcanales vcanales left a comment

Choose a reason for hiding this comment

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

This change is OK to fix the error, but I'm now noticing that each panel is pulling theme data individually.

I think we should consider getting the data on the parent and share the data down, so that we may avoid repeatedly fetching the theme data, what do you think? (For another time, not now)

@pbking
Copy link
Contributor Author

pbking commented May 23, 2024

I think we should consider getting the data on the parent and share the data down, so that we may avoid repeatedly fetching the theme data, what do you think? (For another time, not now)

I'm unconcerned about the performance of repeated calls. I would only want to refactor if it makes the code simpler or better since it ultimately doesn't effect the user's experience. That said, I do think that consolidating all of the data fetching logic (into resolvers) and cacheing that so that it's not fetched every time would both improve the code and minimize the data.

@pbking pbking merged commit 244cb1c into trunk May 23, 2024
2 checks passed
@pbking pbking deleted the fix/metadata-modal-console-error branch May 23, 2024 15:23
@vcanales
Copy link
Member

I'm unconcerned about the performance of repeated calls. I would only want to refactor if it makes the code simpler or better since it ultimately doesn't effect the user's experience.

Yeah, as you say, my concern is not about performance, but about the repetition of code in multiple children of a component that could do the fetching for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants