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

Hide empty message box if there isn't text #6151

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

viown
Copy link
Member

@viown viown commented Oct 1, 2024

Changes
Hide the message box when there won't be text.

Issues
Fixes #6144

@viown viown requested a review from a team as a code owner October 1, 2024 18:23
@@ -87,7 +87,7 @@ function getEditorHtml(options, systemInfo) {
let html = '';
html += '<div class="formDialogContent scrollY">';
html += '<div class="dialogContentInner dialog-content-centered" style="padding-top:2em;">';
if (!options.pathReadOnly) {
if (!options.pathReadOnly && (options.instruction || systemInfo.OperatingSystem)) {
Copy link
Member

Choose a reason for hiding this comment

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

The operating system will never be set as it was removed from the system info in 10.9:

jellyfin/jellyfin#9175

Copy link

sonarcloud bot commented Oct 3, 2024

@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 725c24a4786b387ccf31b65bf2cdf4c74492861c
Status ✅ Deployed!
Preview URL https://c7fe4b91.jellyfin-web.pages.dev
Type 🔀 Preview

Comment on lines -1085 to -1086
"MessageDirectoryPickerBSDInstruction": "For BSD, you may need to set up storage within your 'FreeNAS Jail' so Jellyfin can access your media.",
"MessageDirectoryPickerLinuxInstruction": "For Linux on Arch Linux, CentOS, Debian, Fedora, openSUSE, or Ubuntu, you must grant the service user at least read access to your storage locations.",
Copy link
Member

Choose a reason for hiding this comment

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

Now that I see what these help messages should've shown, it might be worth combining these two texts into one and always displaying that? Kinda useful to have in-app help.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps the text is too long for an info banner? Maybe we can get away with removing the mentioned distros part.
image

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we can have a small text and link to a (new) documentation page with a more extensive explanation?

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

Successfully merging this pull request may close these issues.

Emtpy message box showing in "select path" popup
4 participants