fix: Removed Hardcoded Locales and Improved Error Handlings #2125
Conversation
vijayaragavan462
changed the title
|
Hi team, This pull request is high-priority. Could you please review it at your earliest convenience? Your timely attention to this pull request would be greatly appreciated. Thank you! |
| setMissedField(missedObject); | ||
| } | ||
|
|
||
| }, []) |
There was a problem hiding this comment.
Is it intentional that the dependency array is empty?
There was a problem hiding this comment.
Yes, It will happen only once initially.
| if (url) { | ||
| checkImageURL(url, function(isValid:boolean) { | ||
| if (isValid) { | ||
| console.log('The image URL is valid.'); |
There was a problem hiding this comment.
Is this console log still needed here?
There was a problem hiding this comment.
Not needed anymore
| setSelectedImage(sdk.entry.fields.title.getValue()) | ||
| setImageStatus(true) | ||
| } else { | ||
| console.log('The image URL is invalid.'); |
There was a problem hiding this comment.
Same question: Is this console log still needed here?
| @@ -349,7 +349,7 @@ body { | |||
| } | |||
|
|
|||
| .linkUploadImage > img { | |||
| width: 100%; | |||
| /* width: 100%; */ | |||
There was a problem hiding this comment.
Should this commented code be removed?
There was a problem hiding this comment.
Yes of course, it can be removed
| } | ||
|
|
||
| setDefaultLocale(sdk?.locales?.default); | ||
| }, []) |
There was a problem hiding this comment.
Same question about the dependency array: is it intentionally empty?
There was a problem hiding this comment.
Yes, intentionally.
| <img src={fieldMissing} alt="Field Missing" style={{ height: "100%" }} /> | ||
| </div> | ||
| <div> | ||
| It seems you are missed some fields in this content type which is required to use this custom application. |
There was a problem hiding this comment.
Consider writing this a little more clearly. "It seems you have missed some fields..."
There was a problem hiding this comment.
Sure , will do that.
| size="small" | ||
| contentType="Fields Missing" | ||
| title={`Name of fields : ${missedField.map((x: any) => x.name)}`} | ||
| description="Verify the field type should be 'Short text field' in Content model" |
There was a problem hiding this comment.
Similarly you may want to rewrite this to say "... in the Content model"
.contentful/.circleci/config.yml
Outdated
There was a problem hiding this comment.
This file needs to be removed
|
Hi @david-shibley-contentful , |
.circleci/config.yml
Outdated
There was a problem hiding this comment.
looks like you removed the wrong file
|
@vijayaragavan462 we need you to remove |
Sure @david-shibley-contentful , I have corrected those changes. |
Purpose
Added some more error handling to the application when some fields are removed from the content model and fixed some bugs reported by users
Approach
When there is fields that are missing which was required to use this application we shown a message to the user what was the exact issue is.
Testing steps
If the user deletes some of the fields from the content model, it will show the actual issue that has to be fixed.