changed the href to fix the problem #856
Conversation
sohammk08
changed the title
Added a test in DomainLink.test.ts that checks if the domain being provided is HEX-encoded
|
I ran it on my machine and it passes all the tests without raising any additional issues. Please take a look. |
updates the DomainLink component to handle domain protocols more effectively. It includes changes to check for the presence of protocols (http:// or https://), improving the handling of decoded domains, and ensuring the proper construction of href attributes.
…nent refactored the DomainLink component by enhancing the protocol detection logic, now using a pre-defined regex 'protocolRegex' for improved efficiency
| const wrapper = mount( | ||
| <DomainLink decode domain="68747470733A2F2F6578616D706C652E636F6D" />, | ||
| ) | ||
| expect(wrapper.find('a').props().className).toEqual('domain') | ||
| expect(wrapper.find('a').text()).toEqual('https://example.com') | ||
| expect(wrapper.find('a').props().href).toEqual('https://example.com') | ||
| wrapper.unmount() | ||
| }) |
There was a problem hiding this comment.
This is a bit more understandable:
| const wrapper = mount( | |
| <DomainLink decode domain="68747470733A2F2F6578616D706C652E636F6D" />, | |
| ) | |
| expect(wrapper.find('a').props().className).toEqual('domain') | |
| expect(wrapper.find('a').text()).toEqual('https://example.com') | |
| expect(wrapper.find('a').props().href).toEqual('https://example.com') | |
| wrapper.unmount() | |
| }) | |
| const url = 'https://example.com' | |
| const urlHex = encodeHex(url) | |
| const wrapper = mount( | |
| <DomainLink decode domain={urlHex} />, | |
| ) | |
| expect(wrapper.find('a').props().className).toEqual('domain') | |
| expect(wrapper.find('a').text()).toEqual(url) | |
| expect(wrapper.find('a').props().href).toEqual(url) | |
| wrapper.unmount() | |
| }) |
There was a problem hiding this comment.
As for the additional test that is created in the DomainLink.test.tsx, I just followed the format of the previous tests in the same test file. If only this test is changed, the file will be non-uniform. If you still want to change it the way you mentioned above, let me know.
There was a problem hiding this comment.
That's a good point. I'd still change this test - readability >> consistency for minor things like this. If I were writing this PR I'd modify all the old tests as well, but you don't have to do that.
There was a problem hiding this comment.
Changed it for better readability: b8fc786
High Level Overview of Change
Slightly changed the href in the DomainLink.tsx file. Addresses issue #714
Context of Change
Some accounts such as the issuers of NFTs put fully formatted urls in the domain field. Since "https://" is prepended to the value of that field, an invalid URI is created. The change adds "https://" only if a protocol is not detected.
Type of Change
TypeScript/Hooks Update
Before / After
Before: "https://" was prepended to the value of that field, creating a invalid URL
After: Now, "https://" is added only if a protocol is not detected.
Test Plan
No test plan in practice for the current change.
Note
This is the example given in the issue: https://livenet.xrpl.org/accounts/r481fwr9G6GqJsnDTRanb9wicGjFf3rfgp/transactions
For some reason, whenever I click the DOMAIN on the accounts page(of the example), it leads to:
When it should lead to this, instead:
I suspect that there might be a problem with the data being passed to the DomainLink component. I have not been able to pinpoint it though.
Further feedback is much appreciated.
Thanks you :)