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

fix(AdminSettings): migrate to NcSettingsSection #12840

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Jul 29, 2024

☑️ Resolves

  • Migrate to Nextcloud common NcSettingsSection for design consistency
    • Also, add doc links
  • Missing features:
    • Description slot for description with links or multi-line description
    • BETA badge

🖌️ UI Checklist

🖼️ Screenshots / Screencasts

🏚️ Before 🏡 After
Screenshot before Screenshot after

🏁 Checklist

  • 🌏 Tested with Chrome, Firefox and Safari or should not be risky to browser differences
  • 🖥️ Tested with Desktop client or should not be risky for it
  • 🖌️ Design was reviewed, approved or inspired by the design team
  • ⛑️ Tests are included or not possible
  • 📗 User documentation in https://github.com/nextcloud/documentation/tree/master/user_manual/talk has been updated or is not required

@ShGKme ShGKme added 2. developing design feature: settings ⚙️ Settings and config related issues feature: frontend 🖌️ "Web UI" client labels Jul 29, 2024
@ShGKme ShGKme added this to the 💙 Next Beta (30) milestone Jul 29, 2024
@ShGKme ShGKme self-assigned this Jul 29, 2024
@ShGKme ShGKme force-pushed the fix/admin-settings--migrate-to-NcSettingsSection branch from 125a534 to 702f3b7 Compare July 29, 2024 10:30
class="federation"
:name="t('spreed', 'Federation')"
doc-url="https://docs.nextcloud.com/server/latest/user_manual/en/talk/advanced_features.html#federated-conversation">
<!-- <small>{{ t('spreed', 'Beta') }}</small> -->
Copy link
Member

Choose a reason for hiding this comment

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

Add to name then?

Copy link
Contributor Author

@ShGKme ShGKme Jul 29, 2024

Choose a reason for hiding this comment

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

Then it loses all the "badge" styles

class="matterbridge"
:name="t('spreed', 'Matterbridge integration')"
doc-url="https://nextcloud-talk.readthedocs.io/en/latest/matterbridge/">
<!-- <small>{{ t('spreed', 'Beta') }}</small> -->
Copy link
Member

Choose a reason for hiding this comment

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

🙈

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • Missing features:
    • BETA badge

Requires some adjustments on NcSettingsSection 😶

@ShGKme ShGKme force-pushed the fix/admin-settings--migrate-to-NcSettingsSection branch from 702f3b7 to 6e5ca59 Compare July 29, 2024 13:28
<NcSettingsSection id="stun_server"
:name="t('spreed', 'STUN servers')"
:description="t('spreed', 'A STUN server is used to determine the public IP address of participants behind a router.')"
doc-url="https://nextcloud-talk.readthedocs.io/en/latest/TURN/">
Copy link
Member

Choose a reason for hiding this comment

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

stun is not turn?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not, but on the TURN page (the "configuration/overview" page) there is a small paragraph describing what is STUN and when it is necessary.

  • Talk tries to establish a direct peer-to-peer (P2P) connection, thus on connections beyond the local network (behind a NAT or router), clients do not only need to know each other's public IP, but the participants local IPs as well. Processing this, is the job of a STUN server. As there is one preconfigured for Nextcloud Talk that is operated by Nextcloud GmbH, for this case nothing else needs to be done.

Better to link to a Wiki article directly?

Copy link
Member

Choose a reason for hiding this comment

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

I would say either it should be a useful installation/setup/configuration manual or we don't add a link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants