Skip to content

Polish import/settings UI and fix Share Importer composer #207

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

Merged
dprodger merged 1 commit into from
May 31, 2026
Merged

Polish import/settings UI and fix Share Importer composer #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 11 additions & 40 deletions apps/Mac/Views/Import/MacSongCreationView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ struct MacSongCreationView: View {
@State private var composer: String
@State private var musicbrainzId: String
@State private var workType: String
@State private var key: String

@State private var isSaving = false
@State private var showingError = false
Expand All @@ -33,30 +32,14 @@ struct MacSongCreationView: View {
_composer = State(initialValue: importedData?.composerString ?? "")
_musicbrainzId = State(initialValue: importedData?.musicbrainzId ?? "")
_workType = State(initialValue: importedData?.workType ?? "")
_key = State(initialValue: importedData?.key ?? "")
}

var body: some View {
VStack(spacing: 0) {
// Header
HStack {
Button("Cancel") {
dismiss()
}
.keyboardShortcut(.cancelAction)

Spacer()

Text("Create Song")
.font(ApproachNoteTheme.headline())

Spacer()

Button("Save") {
saveSong()
}
.keyboardShortcut(.defaultAction)
.disabled(title.isEmpty || isSaving)
}
.padding()

Expand All @@ -70,37 +53,25 @@ struct MacSongCreationView: View {

TextField("Composer", text: $composer)
.textFieldStyle(.roundedBorder)

TextField("MusicBrainz ID", text: $musicbrainzId)
.textFieldStyle(.roundedBorder)
.font(.system(.body, design: .monospaced))
} header: {
Text("Basic Information")
}

Section {
TextField("Work Type (e.g., Song, Instrumental)", text: $workType)
.textFieldStyle(.roundedBorder)

TextField("Key (e.g., Eb, F minor)", text: $key)
.textFieldStyle(.roundedBorder)
} header: {
Text("Additional Details")
}

Section {
Text("Additional details (structure, recordings) can be added later through the app.")
.font(ApproachNoteTheme.caption())
.foregroundColor(.secondary)
}
}
.formStyle(.grouped)
.padding()

if isSaving {
ProgressView("Saving...")
.padding()
// Footer buttons
VStack(spacing: 12) {
ApproachNoteButton("Create Song", isLoading: isSaving, action: saveSong)
.keyboardShortcut(.defaultAction)
.disabled(title.isEmpty || isSaving)

ApproachNoteButton("Cancel", style: .secondary) {
dismiss()
}
.keyboardShortcut(.cancelAction)
}
.padding()
}
.frame(minWidth: 400, minHeight: 400)
.alert("Error", isPresented: $showingError) {
Expand Down
6 changes: 4 additions & 2 deletions apps/ShareImporter/CommonViews.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,16 @@ struct NotImplementedView: View {
struct InfoRow: View {
let label: String
let value: String

var deEmphasized: Bool = false

var body: some View {
VStack(alignment: .leading, spacing: 4) {
Text(label)
.font(.caption)
.foregroundColor(.secondary)
Text(value)
.font(.body)
.font(deEmphasized ? .caption.monospaced() : .body)
.foregroundColor(deEmphasized ? .secondary : .primary)
}
}
}
122 changes: 100 additions & 22 deletions apps/ShareImporter/ExtractMusicBrainzData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,17 +155,42 @@ ExtractMusicBrainzData.prototype = {
title = pageTitle.replace(/\s*-\s*MusicBrainz\s*$/, '').trim();
}

// Extract composer(s) - try multiple selectors
var composers = [];

// Look for composer or writer in the details list
var composerElements = document.querySelectorAll('dd.writer a[href*="/artist/"], dd.composer a[href*="/artist/"]');
composerElements.forEach(function(element) {
var composerName = element.textContent.trim();
if (composerName && composers.indexOf(composerName) === -1) {
composers.push(composerName);
// Composer(s) are filled in from the MusicBrainz web service below
// (see the artist-rels fetch before completionFunction). The DOM is an
// unreliable source because the relationship markup varies and the work
// page is increasingly served behind a JS challenge. We still keep a
// best-effort DOM scrape as a fallback if the API call fails.
function scrapeComposersFromDOM() {
var found = [];
function addFromDd(dd) {
if (!dd) return;
dd.querySelectorAll('a[href*="/artist/"]').forEach(function(element) {
var name = element.textContent.trim();
if (name && found.indexOf(name) === -1) {
found.push(name);
}
});
}
});
document.querySelectorAll('dt').forEach(function(dt) {
var label = dt.textContent.trim().toLowerCase();
if (label.indexOf('composer') !== -1 || label.indexOf('writer') !== -1) {
var sibling = dt.nextElementSibling;
while (sibling && sibling.tagName === 'DD') {
addFromDd(sibling);
sibling = sibling.nextElementSibling;
}
}
});
if (found.length === 0) {
document.querySelectorAll('dd.writer a[href*="/artist/"], dd.composer a[href*="/artist/"]').forEach(function(element) {
var name = element.textContent.trim();
if (name && found.indexOf(name) === -1) {
found.push(name);
}
});
}
return found;
}

// Extract type (song, instrumental, etc.)
var workType = "";
Expand Down Expand Up @@ -220,36 +245,89 @@ ExtractMusicBrainzData.prototype = {
};

// Only add optional fields if they have values
if (composers.length > 0) {
result.composers = composers;
}

if (workType) {
result.workType = workType;
}

if (musicalKey) {
result.key = musicalKey;
}

if (iswc) {
result.iswc = iswc;
}

if (language) {
result.language = language;
}

if (annotation) {
result.annotation = annotation;
}

if (wikipediaUrl) {
result.wikipediaUrl = wikipediaUrl;
}

// Return results to the extension
arguments.completionFunction(result);

// Resolve composers from the MusicBrainz web service (authoritative),
// then return results to the extension. completionFunction must be
// called exactly once, so guard the timeout/fetch race. Alias the
// extension args because `arguments` is shadowed inside the callbacks
// by their own implicit arguments object.
var extensionArgs = arguments;
var completed = false;
function finish(composerList) {
if (completed) return;
completed = true;
if (composerList && composerList.length > 0) {
result.composers = composerList;
}
extensionArgs.completionFunction(result);
}

// Safety net: never let a hung request strand the import.
var timeoutId = setTimeout(function() {
finish(scrapeComposersFromDOM());
}, 4000);

if (musicbrainzId) {
var apiUrl = "https://musicbrainz.org/ws/2/work/" + musicbrainzId +
"?inc=artist-rels&fmt=json";
fetch(apiUrl, { headers: { "Accept": "application/json" } })
.then(function(response) {
return response.ok ? response.json() : null;
})
.then(function(json) {
clearTimeout(timeoutId);
var names = [];
if (json && json.relations) {
// Prefer "composer"; fall back to writer/lyricist only
// if no composer relationship exists.
var rels = json.relations.filter(function(rel) {
return rel.type === "composer";
});
if (rels.length === 0) {
rels = json.relations.filter(function(rel) {
return rel.type === "writer" || rel.type === "lyricist";
});
}
rels.forEach(function(rel) {
var name = rel.artist && rel.artist.name ? rel.artist.name.trim() : "";
if (name && names.indexOf(name) === -1) {
names.push(name);
}
});
}
finish(names.length > 0 ? names : scrapeComposersFromDOM());
})
.catch(function() {
clearTimeout(timeoutId);
finish(scrapeComposersFromDOM());
});
} else {
clearTimeout(timeoutId);
finish(scrapeComposersFromDOM());
}
},

extractYouTubeData: function(arguments, url) {
Expand Down
12 changes: 3 additions & 9 deletions apps/ShareImporter/SongViews.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ struct SongImportConfirmationView: View {
var body: some View {
VStack(spacing: 20) {
VStack(spacing: 12) {
Image(systemName: "music.note")
.font(.system(size: 50))
.foregroundColor(.blue)

Text("Import Song")
.font(.title2)
.bold()
Expand All @@ -46,13 +42,11 @@ struct SongImportConfirmationView: View {
InfoRow(label: "Key", value: key)
}

InfoRow(label: "MusicBrainz ID", value: songData.musicbrainzId)
InfoRow(label: "MusicBrainz ID", value: songData.musicbrainzId, deEmphasized: true)
}
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
.padding(.horizontal)
}
.background(Color(.systemGray6))
.cornerRadius(12)
.padding(.horizontal)

Spacer()

Expand Down
16 changes: 6 additions & 10 deletions apps/ShareImporterMac/MacExtensionViews.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,16 @@ struct MacNotImplementedView: View {
struct MacInfoRow: View {
let label: String
let value: String
var deEmphasized: Bool = false

var body: some View {
VStack(alignment: .leading, spacing: 4) {
Text(label)
.font(.caption)
.foregroundColor(.secondary)
Text(value)
.font(.body)
.font(deEmphasized ? .caption.monospaced() : .body)
.foregroundColor(deEmphasized ? .secondary : .primary)
}
}
}
Expand Down Expand Up @@ -486,10 +488,6 @@ struct MacSongImportConfirmationView: View {
var body: some View {
VStack(spacing: 16) {
VStack(spacing: 8) {
Image(systemName: "music.note")
.font(.system(size: 40))
.foregroundColor(.blue)

Text("Import Song")
.font(.title2)
.bold()
Expand All @@ -516,13 +514,11 @@ struct MacSongImportConfirmationView: View {
MacInfoRow(label: "Key", value: key)
}

MacInfoRow(label: "MusicBrainz ID", value: songData.musicbrainzId)
MacInfoRow(label: "MusicBrainz ID", value: songData.musicbrainzId, deEmphasized: true)
}
.padding()
.frame(maxWidth: .infinity, alignment: .leading)
.padding(.horizontal)
}
.background(Color(NSColor.controlBackgroundColor))
.cornerRadius(8)
.padding(.horizontal)

Spacer()

Expand Down
13 changes: 1 addition & 12 deletions apps/iOS/Views/SettingsView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,19 +268,8 @@ struct SettingsView: View {
}
.padding(.horizontal)
} else {
Button(action: {
ApproachNoteButton("Sign In / Sign Up") {
isShowingLogin = true
}) {
HStack {
Image(systemName: "person.crop.circle.badge.plus")
.foregroundColor(ApproachNoteTheme.brand)
Text("Sign In or Create Account")
.foregroundColor(ApproachNoteTheme.textPrimary)
Spacer()
}
.padding()
.background(ApproachNoteTheme.surface)
.cornerRadius(8)
}
.padding(.horizontal)
}
Expand Down
Loading
Loading