-
Notifications
You must be signed in to change notification settings - Fork 466
feat: Update Enum Type descriptions #3355
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
feat: Update Enum Type descriptions #3355
Conversation
- add new methods for getting enum descriptions and added new graphql_pre_enum_description filter
justlevine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand the purpose of this PR.
- why do we need/want a filter just for the enum descriptions?
- why is that a pre filter?
- whats the difference between a description and a default description?
- why is it a trait?
…PEnumType class - move switch statements inline instead of using `get_default_description` method from the trait
| * @param string $size Optional. The size to get information for. | ||
| * @return array<string, array{width: int, height: int, crop: bool}>|null | ||
| */ | ||
| protected static function get_image_sizes( $size = '' ): ?array { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function get_image_sizes has a Cognitive Complexity of 8 (exceeds 5 allowed). Consider refactoring.
graphql_pre_enum_description filter, and update Enum descriptions
- cleanup debug code
graphql_pre_enum_description filter, and update Enum descriptions|
@justlevine thanks for the review.
As I was working on making descriptions more clear, I realized that a lot of the descriptions might require more context in order for plugins to be able to filter a description should they want to and provide accurate context. For example, take the MediaItemSize enum.
While I think the new descriptions are more clear overall, I do think that some applications will likely want to override these default descriptions with more clarity for their specific intentions for each size. For example a restaurant website might want to say that any image with xx width should be used for food item listings. Or a newspaper might say that the image with xx width should be used on the home page. Or whatever the use case might be, some extra context might be needed for the filter to provide proper response. In order to provide that level of specificity, a plugin might need additional context to filter the description such as the name of the image size, the height, the width, etc. (or whatever context relevant to the specific Enum Type). I added the new Ultimately, I think I over-complicated it, and thanks to your challenge, I decided to scale it back. I've removed the trait as I think ultimately it was applied at too-high of a level anyway. It was implemented as a trait to make the code a bit easier to share across classes that weren't implementing another parent class. I could have added an abstract class to implement, but this felt simpler at the time, but really I think it ended up being more complicated. The existing |
|
Code Climate has analyzed commit e5a9fe0 and detected 6 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Description
This PR focuses on updating descriptions for Enum types to be more clear.
New Features:
graphql_enum_valuesfilter(s) (see: https://github.com/wp-graphql/wp-graphql/pull/3355/files#diff-ce95ce616e43f99d34ed9d1d3b527ca28dfce56b10ab9e4946dccf8bfd741653R79-R97)Updates:
Most of the changes in this PR are updates to the descriptions of Enum Types and other fields.
Here's a preview of the before/after:
<body>tag<body>tag. (allows content to render first)<head>tag<head>tag. (executes before page content renders)asyncattributedeferattribute