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

Add an example for Navigator.userAgent #2347

Closed
wants to merge 3 commits into from

Conversation

MrBrain295
Copy link
Contributor

No description provided.

@MrBrain295
Copy link
Contributor Author

@NiedziolkaMichal

@NiedziolkaMichal
Copy link
Member

Thank you for the PR. Unfortunately webapi-tabbed type is not yet ready to be used in production. Please refer to #2306 and #15166 for an explanation.

@MrBrain295
Copy link
Contributor Author

Hmmm, should I close this or mark it as a draft?

@NiedziolkaMichal NiedziolkaMichal marked this pull request as draft December 3, 2022 17:06
@NiedziolkaMichal
Copy link
Member

Draft will do fine. I will notify you when something changes.

@MrBrain295
Copy link
Contributor Author

@NiedziolkaMichal I don’t know what to do about meta.json but from what I saw in mdn/mdn-community#305 this should be good now.

@NiedziolkaMichal
Copy link
Member

Since your example requires just a JS tab, you can use the JS editor. Your meta.json content should be something like that:

        "navigatorPrototypeUserAgent": {
            "exampleCode": "./live-examples/webapi-examples/navigator/navigator-prototype-useragent.js",
            "fileName": "navigator-prototype-useragent.html",
            "title": "Web API Demo: Navigator.prototype.userAgent",
            "type": "js"
        }

You would need to move and rename js file to fit exampleCode value. Still, there are a few more problems with it:

  • JS editor breaks when output is too long. I think it is fixed in the next BOB version, so we would need to wait for it to be deployed.
    image
  • Pretty much every usage of console.log in JS examples, has comment about expected output directly below. For example // expected output: [6, 6, 6, 6]. Here output depends on the browser of a user, so maybe example output would be more fitting. Still, any output is pretty big and it would show a scrollbar in the editor:
    image
  • You might have read that @wbamberg doesn't see Navigator.userAgent as a good candidate for an interactive example. You would need to convince him otherwise.

@wbamberg
Copy link
Contributor

You might have read that @wbamberg doesn't see Navigator.userAgent as a good candidate for an interactive example. You would need to convince him otherwise.

The point about interactive examples is that they are interactive: the user can edit them and see something different. So for example the one for slice() is nice: it's actually quite a fiddly API, and the example gives you a way to try it out with different parameters and see the effect. If a user is actually trying to figure out how to use it with a particular array they can paste that in too and try out different params until they get the answer they want.

But this example isn't interactive - there's nothing you can do with the editor: all you can do is press "Run". You could implement this as a normal live sample. And all it will tell you is your browser's UA string. It would be much more helpful to users to list several common UA strings.

When we started the interactive examples project we were conscious of the fact that they are quite rude: they occupy quite a lot of the most precious space on the page. So we always felt that there would be features where it wasn't appropriate to add examples, because they didn't add enough value to justify their existence - and I would say this is one of these cases. So I think it's only worth adding examples like this if we want to change that policy, and instead want to add examples for just about everything.

@MrBrain295 MrBrain295 closed this Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants