search

deprecate / metal-uri

Class for parsing and formatting URIs.
Other
3 stars 7 forks source link

Rebase from vbence86:feature/improved-parsing #18

Closed vbence86 closed 7 years ago

vbence86 commented 7 years ago

Intent

To provide more flexible parsing functionality that works with major built-in <a> URI validations.

jbalsas commented 7 years ago

Hey @brunobasto, @Robert-Frampton, @eduardolundgren could you guys take a look at this? We're adding a stronger support for our fallback so it behaves similarly to modern browsers.

Do you think it is worth adding this complexity or should we simply drop support or assume a slightly flunky behaviour on browsers not supporting URL?

jbalsas commented 7 years ago

Hey @vbence86, I just realized we have https://github.com/metal/metal-useragent, maybe we can use that instead of a custom uaHelper here?

vbence86 commented 7 years ago

Though metal-useragent indeed exposes a function to check whether the browser is Safari, the implementation still requires a functionality to check the version. Do you want me to delegate it to metal-useragent and rid the code of the uaHelper fully?

or

Keep using the uaHelper with empoying the isSafari function from metal-useragent.

vbence86 commented 7 years ago

I've updated the function in order to exclude auth sections as potential port numbers.

captura de pantalla 2017-09-18 a las 12 06 25
eduardolundgren commented 7 years ago

Thanks for working on those improvements @vbence86.

@jbalsas @brunobasto I won't be able to analyze it deeply this week, you guys can take the decision. Keep in mind that this module is used everywhere by our other dependencies, can't break :)

jbalsas commented 7 years ago

All tests and more are passing, so that should be a good indicator... in any case, we can still run the tests on a local link for:

@vbence86, could you take care of that?

vbence86 commented 7 years ago

No problem, will report the results in due course.

vbence86 commented 7 years ago

All tests against the pointed projects have been successfully executed locally using metal-uri:travis branch. As a reminder, please note that my local testing environment only employs Chrome.

captura de pantalla 2017-09-20 a las 10 44 41 captura de pantalla 2017-09-20 a las 10 45 03 captura de pantalla 2017-09-20 a las 10 48 22

jbalsas commented 7 years ago

Awesome @vbence86!!

@Robert-Frampton, can you take a look at this one and merge or ask for additional changes?

robframpton commented 7 years ago

Hey guys,

LGTM. I think in regards to leveraging metal-useragent, we should add some more functionality for checking browser versions. Once that's in place we can remove the uaHelper from this repo.