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

Remove wss:// from relay lists and allow entry of relay without wss:// #1417

Merged
merged 12 commits into from
Aug 23, 2024
Merged

Remove wss:// from relay lists and allow entry of relay without wss:// #1417

merged 12 commits into from
Aug 23, 2024

Conversation

lingoslinger
Copy link
Contributor

Issues covered

1401

Description

Made changes to remove the wss:// from relay lists and changes to allow the entry of relays without the wss://. Also modified the relayAddressPlaceholder localizable string.

How to test

  • navigate to the relay list
  • observe that there are no longer any wss:// references
  • add a relay without using a prepended wss:// and observe that it is added successfully

Screenshots/Video

  • before changes:
    1401-before
  • after changes:
    1401-after

joshuatbrown
joshuatbrown requested changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Just a few suggestions on implementation. Works as expected, and this is really nice to have!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Nos/Assets/Localization/Localizable.xcstrings Outdated Show resolved Hide resolved
Nos/Models/CoreData/Relay+CoreDataClass.swift Outdated Show resolved Hide resolved
Nos/Views/RelayView.swift Outdated Show resolved Hide resolved
joshuatbrown
joshuatbrown requested changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more small changes to make.

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Nos/Assets/Localization/Localizable.xcstrings Outdated Show resolved Hide resolved
Nos/Views/RelayView.swift Outdated Show resolved Hide resolved
joshuatbrown
joshuatbrown approved these changes Aug 23, 2024
View reviewed changes
Copy link
Contributor

@joshuatbrown joshuatbrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work, and thanks again, @lingoslinger!

@joshuatbrown joshuatbrown added this pull request to the merge queue Aug 23, 2024
Merged via the queue into planetary-social:main with commit 9086cdb Aug 23, 2024
4 checks passed
@lingoslinger lingoslinger deleted the 1401-hide-wss-in-relay-list branch August 23, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuatbrown joshuatbrown joshuatbrown approved these changes

@mplorentz mplorentz Awaiting requested review from mplorentz mplorentz is a code owner

@martindsq martindsq Awaiting requested review from martindsq

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lingoslinger @joshuatbrown