Feedback for Wiki Viewer

Code Feedback
1

As far as i can tell its been removed from the curriculum but I’ve just finished :joy:
So I’ve made the project responsive, mobile first.
Any feedback would be fantastic.


24
242880×1604 1.58 MB

Thanks in advance :grinning:

2

Hi @MattyGlen123,

HTML

line 9:

The element “button” must not appear as a descendant of the “a” element.

   <a id="random"  href="https://en.wikipedia.org/wiki/Special:Random" target="_blank"><button class="random-button"><i class="fas fa-random"></i> Random</button></a>

MDN documentation:

Permitted content:
Transparent, containing either flow content (excluding interactive content) or phrasing content.

Content categories:
Flow content, phrasing content, Interactive content, listed, labelable, and submittable form-associated element, palpable content.

Interactive content includes elements that are specifically designed for user interaction. Elements that belong to this category include: <button> …

Testing

  1. Test: No input in “search bar”, click button “Search” :
    Result: No visible change

noString.png800×600 467 KB

  1. Test: invalid input (string) in “search bar”, click button “Search”:
    Result: A white rectangle at the bottom, after the “My Portfolio” section:

randomString.png800×600 357 KB

3.Test: Input: “gundam” (string), click button “Search”:
Result: Expected behavior

gundamSearch.png800×1801 658 KB

Testing recommendation

  1. Test: No input in “search bar”, click button “Search” :
  • Add a message asking for an input
  1. Test: invalid input (string) in “search bar”, click button “Search”:
  • Replace withe rectangle at the bottom with valid html section

Cheers and happy coding :slight_smile:

Note:
Tools used:

1 Like
3

Hey erretres,
Thank you so much for the feedback, i’ve only just read through it but I really appreciate such structured information. I’m definitely going to implement all your suggestions!
thanks again :slight_smile:

1 Like
4

Design is a little heavy I think, this could be lighter, but layout works.

as background image is kind of cover, it scale(move) based on result(when you populate the result panel), I think it’s not good.

search text field is responsible for enter key, very good.

One issue is there is no loading indicator. When a request is made, this could be some loading icon/stuff to inform user the request in under progress.

The error message is at debug level. informing user using alert is not very appreciated, use some better way.

Another bug is you do not escape the requested by user to call the API, this is bad. just try “?” or “&” and see what happens. Fix it.

keep goin on good work, happy programming

5

Hi Null_dev,
Thanks for the feedback, I’ve been working through your suggestions and i’m afraid i don’t understand what you mean when you say “Another bug is you do not escape the requested by user to call the API, this is bad. just try “?” or “&” and see what happens. Fix it.”.
I’m guessing this is something to do with my query string? could you elaborate for me please?
Thanks again for the feedback.

6

Your code to prepare the URl for calling the API as following
"https://en.wikipedia.org/w/api.php?action=opensearch&search=" + $('#userInput').val() + "&format=json&callback=?"
So now what happens when user enter & ?
it will be something like
https://en.wikipedia.org/w/api.php?action=opensearch&search=&&format=json&callback=?
the part search=& doesn’t mean search for &, it means search for nothing, and & here means proceed to next parameter, it’s clear?

So simple, you should either escape it(old way), or let the API add it by setting the data, check following (line 9)

/*...*/
url: "https://en.wikipedia.org/w/api.php?action=opensearch&format=json&callback=?",
        data:{"search":$('#userInput').val()},
/*...*/

Instead of setting the parameter and value by URL, let local JS framework(Jquery here) does it for you, it escapes it automatically.

Note: if you like to specify two or more parameter with a same name, jquery might ignore second and beyond, you may go for old escape form instead which will work)

Now with the fix search for & or ? and see the difference.

keep goin on great work, happy programming

1 Like
7

Yep, I understand what you mean now, thanks for the explanation :grinning:
I’ve changed a few things, got rid of the background img.
I implemented a loading indicator, first one i ever done so that was a really good experience :yum: and changed the error message.

I think it passes all your testing recommendation now erretres.

Cheers for the help both of you, anything else let me know.
https://codepen.io/MattyGlen/pen/MGZZwa

43
432880×1630 193 KB

8

O yes, now looks much better, yes plain design always win, very good.

One thing you may keep it in mind, this is not critical, and could be tricky to solve, but telling you to think about it and maybe someday you could do something for this.

As my experience in programming over years, I would say the trickiest(there is no hard part) part of programming is the state of the program. To keep it right, sometimes it’s a little tricky.

Note: not sure if your API call is actually non-async,(I cannot see any sync behavior such as blocked UI response) please read here.

Assume time is 0 (zero second)
For you page, assume user starts search for “A”, then script starts to call the API and you show the loading.(that’s correct for now)
Now assume for some reason(example bad network) the API calls takes around 30 seconds.(so user needs to wait for 30 seconds)
But user decides to start search another thing after 10 seconds( now time is 10 ,20 seconds reminded to load “A”), this time “V” for instance. Assume “V” search going to take 10 seconds.
Now what happens?
time 20
You get “V” response on time 20, and show the result, also stopped the loading indicator.
after 10 seconds
time 30
now result of “A” is ready, but user didn’t expected that, but browser got it. now it show the “A” result, just 10 second after did show the “V” result without any sign.

So what is the state issue here:

  1. user asked for new search for “V”, and it means it asked to cancel or ignore the “A” request, but at the end both will be shown, the lucky one will get there first, but there is no lucky one, only “V” should be there, “A” should be ignored.
  2. the best move is cancelling “A” progress once new “V” request is up. Not sure if it’s possible with js/jquery(not js expert), but even if you cannot cancel, it, you can ignore the response once callback, how? your challenge, easy.
  3. even if your policy is showing any search result no matter who was the first(which is non-sense, please don’t), this is better to keep loading live after any data callback ready, to inform user something else is under progress.

Now go for it, try to fix this small issue, with number 2 explained.

One not appreciate stuff that walking on nerves is about following code
$('.search-bar').val('Enter Text..');
Instead of setting the input text, simply use placeholder as 'Enter Text...' , place holder also doesn’t need any js stuff.

keep going on great work dude, happy coding.