techemmy / React-Countries-Finder

Advanced React Side Project
https://countries-finder-tau.vercel.app
0 stars 1 forks source link

Refining error message depending on what errs #1

Closed thierrymarianne closed 4 months ago

thierrymarianne commented 4 months ago

Hi @techemmy,

Related to this original annoucement

I get a Oops before whitelisting the API domain (assumed to be served from https://restcountries.com/) though.

Does it make sense to remove the API requirement on first load so that it could work without network call to restcountries API?

On another hand, flag assets could be base64 encoded for instance See https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs

countries
thierrymarianne commented 4 months ago

Closed since network connectivity might be perceived as the original issue, which i would not like to deepen further.

See further reasoning from original conversation thread from com.linkedin

techemmy commented 4 months ago

Hi @techemmy,

Related to this original annoucement

I get a Oops before whitelisting the API domain (assumed to be served from https://restcountries.com/) though.

Does it make sense to remove the API requirement on first load so that it could work without network call to restcountries API?

On another hand, flag assets could be base64 encoded for instance See https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/Data_URLs

countries

The API requirement is needed on first load because that's where I fetch the countries data.

Also, I didn't have to whitelist the API before using it 🤔. I'm not sure why you had to do that before it worked for you.

techemmy commented 4 months ago

Closed since network connectivity might be perceived as the original issue, which i would not like to deepen further.

See further reasoning from original conversation thread from com.linkedin

Hmm... network issue or maybe your system might not be allowing connection to that URL. Do you think there's another possible reason for this?

thierrymarianne commented 4 months ago

Your assumption is absolutely correct.

However, it felt kind of disappointing to receive an error message from the homepage (even though the issue clearly lies on my end b/c of some specific set up).

May i suggest an alternative approach (i don't doubt what exists today works perfectly already or other solution might cover having the app functioning without calling a third-party API, which could very well not be available at some point in the future - or did I missed the fact that you may be maintaining restcountries.com/ service as well?):

e.g. by [replacing all flags with base64 encoded pictures] [^1] not requiring a call to an external website depending on actual flag media weights to prevent a performance hit on first load, i would have implemented it myself before offering a pull-request and the thing is, at this point, i already have too much on my plate with other topics so i'd rather not add another one to the wip pile.

Altough, I thought it was worth a discussion, hence the opened / closed issue (for the sake of having a better understanding on the project requirements at least on my end).

It still strongly feels to me there is a need for better tooling when it comes to identifying countries, representation and attached geography, perhaps for learning them with flash cards [^2].

Thank you for drawing attention to these difficulties, by creating this project in the first place! 🙏🏼

[^1] Were they .svg files for instance, they could be rendered straight from the HTML file without requiring access to a domain secondary to the main one. Just an opinion. [^2] https://w.wiki/7e4P

thierrymarianne commented 4 months ago

Could a short message be rendered aside Oops message perhaps? Something along the line of

Please note that network access to https://restcountries.com/ is required
so that the application behaves as it was designed.

What do you think, @techemmy? Would it be a good enough tradeoff in terms of visitor experience?

I've opened a pull-request just in case it would be desirable from your perspective…

techemmy commented 4 months ago

Could a short message be rendered aside Oops message perhaps? Something along the line of

Please note that network access to https://restcountries.com/ is required
so that the application behaves as it was designed.

What do you think, @techemmy? Would it be a good enough tradeoff in terms of visitor experience?

I've opened a pull-request just in case it would be desirable from your perspective…

I think it's a better message to be honest. I've merged the PR already 🎉

techemmy commented 4 months ago

Your assumption is absolutely correct.

However, it felt kind of disappointing to receive an error message from the homepage (even though the issue clearly lies on my end b/c of some specific set up).

May i suggest an alternative approach (i don't doubt what exists today works perfectly already or other solution might cover having the app functioning without calling a third-party API, which could very well not be available at some point in the future - or did I missed the fact that you may be maintaining restcountries.com/ service as well?):

e.g. by [replacing all flags with base64 encoded pictures] [^1] not requiring a call to an external website depending on actual flag media weights to prevent a performance hit on first load, i would have implemented it myself before offering a pull-request and the thing is, at this point, i already have too much on my plate with other topics so i'd rather not add another one to the wip pile.

Altough, I thought it was worth a discussion, hence the opened / closed issue (for the sake of having a better understanding on the project requirements at least on my end).

It still strongly feels to me there is a need for better tooling when it comes to identifying countries, representation and attached geography, perhaps for learning them with flash cards [^2].

Thank you for drawing attention to these difficulties, by creating this project in the first place! 🙏🏼

[^1] Were they .svg files for instance, they could be rendered straight from the HTML file without requiring access to a domain secondary to the main one. Just an opinion. [^2] https://w.wiki/7e4P

Oh, I missed this message. No, I don't maintain the restcountries API. I agree with you. I'm open to pull requests in any way you wish to support.

techemmy commented 4 months ago

@thierrymarianne I thought you sent another PR on a better structure for the error messages. I can't seem to find it

thierrymarianne commented 4 months ago

Hey! Only https://github.com/techemmy/React-Countries-Finder/pull/2 was offered.

Not sure what you mean by better structure, afraid to say that i don't have such thing in mind.

techemmy commented 4 months ago

Hey! Only #2 was offered.

Not sure what you mean by better structure, afraid to say that i don't have such thing in mind.

Oh, never mind. I thought I received another PR from you on displaying different messages based on the error.