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.

Sign up for GitHub

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

Jump to bottom

@trezor/connect-popup: hardcoded urls #5326

Closed
mroz22 opened this issue May 4, 2022 · 2 comments · Fixed by #5487
Closed

@trezor/connect-popup: hardcoded urls #5326

mroz22 opened this issue May 4, 2022 · 2 comments · Fixed by #5487
Assignees
@marekrjpolak
Labels
code Code improvements connect Connect API related (ie. fee calculation) connect-popup Connect popup used by 3rd parties

Comments

@mroz22
Copy link
Contributor

mroz22 commented May 4, 2022

Urls in @trezor/suite are imported from constants folder. We try never to use url string directly which is a practice helping us avoid some dead urls as collection of urils saved in one place is easier to review and possibly test.

@trezor/connect-popup still has hardcoded urls.

Actionpoints:

  • extract hardcoded urls in trezor/connect-popup to a single constants file

Bonus action points:

  • this file might be either inside @trezor/connect-popup project or we might consider creating a new package, lets say @trezor/urls that could be shared accross projects and CI tested.
@mroz22 mroz22 added code Code improvements translations Improvement of derived (non-ENG) copy connect Connect API related (ie. fee calculation) connect-popup Connect popup used by 3rd parties labels May 4, 2022
@mroz22 mroz22 mentioned this issue May 4, 2022
Merged
42 tasks
@mroz22 mroz22 removed the translations Improvement of derived (non-ENG) copy label May 4, 2022
@marekrjpolak marekrjpolak mentioned this issue May 25, 2022
Merged
@hynek-jina hynek-jina moved this to 🤝 Needs QA in Issues Suite Jun 16, 2022
@marekrjpolak
Copy link
Contributor

QA:

Please check that external links are working in:

  • Connect popup
  • Landing page
  • Browser detection page
  • Suite (both web and desktop)

@STew790
Copy link
Contributor

STew790 commented Jun 24, 2022

QA OK

  • Connect popup ✔️
  • Landing page ✔️
  • Browser detection page ✔️
  • Suite (both web and desktop) ✔️

@bosomt bosomt moved this from 🤝 Needs QA to ✅ Approved in Issues Suite Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@marekrjpolak marekrjpolak

Labels
code Code improvements connect Connect API related (ie. fee calculation) connect-popup Connect popup used by 3rd parties
Projects
Issues Suite
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Shared @trezor/urls package trezor/trezor-suite
3 participants
@marekrjpolak @mroz22 @STew790