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

Add editor name prompt on startup #157

Merged
merged 35 commits into from
Dec 12, 2023
Merged

Add editor name prompt on startup #157

merged 35 commits into from
Dec 12, 2023

Conversation

GuzzoD
Copy link
Collaborator

@GuzzoD GuzzoD commented Jun 20, 2023

No description provided.

@GuzzoD GuzzoD requested review from dersmon and tkleinke June 20, 2023 11:21
@GuzzoD
Copy link
Collaborator Author

GuzzoD commented Jun 20, 2023

See #151, #152

@@ -131,4 +138,24 @@ export class AppComponent {
document.addEventListener('dragover', event => event.preventDefault());
document.addEventListener('drop', event => event.preventDefault());
}

private async promptEditorName() {
//this.menus.setContext(MenuContext.CONFIGURATION_MODAL);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tkleinke: Übernommen von Projekt Konfiguration. Brauchen wir das? Wenn ja, wie müsste es hier sein?

Copy link
Member

Choose a reason for hiding this comment

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

Ja, das ist wichtig, damit die richtigen Menüs oben angezeigt werden bzw. damit die Menüs auch deaktiviert sind, solange das Modal geöffnet ist.

In diesem Fall wird das Modal ja immer direkt geöffnet, nachdem die Anwendung geladen wurde, d. h. man befindet sich zuerst im Kontext "Default". Dann wechselt man in den Kontext "Modal" und, nachdem das Modal geschlossen wurde, wieder zurück zu "Default".

Also zuerst am Anfang der Funktion:
this.menus.setContext(MenuContext.MODAL);

Und dann ganz am Ende im finally-Block:
this.menus.setContext(MenuContext.DEFAULT);

selector: 'taskbar-editor',
templateUrl: './taskbar-editor.html'
})
/**
Copy link
Member

Choose a reason for hiding this comment

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

Kurzen Kommentar einfügen, was die Component macht.

class="mdi mdi-account-check"
*ngIf="currentEditor !== 'anonymous'"
></span>
<span id="editor-initials" class="nav-link px-2">{{getInitials()}}</span>
Copy link
Member

@dersmon dersmon Sep 26, 2023

Choose a reason for hiding this comment

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

In Angular templates sollte man für die Performance Funktionsaufrufe vermeiden. Siehe z.B. https://medium.com/showpad-engineering/why-you-should-never-use-function-calls-in-angular-template-expressions-e1a50f9c0496

Comment on lines 145 to 163
public async promptEditorName() {
this.menus.setContext(MenuContext.MODAL);

try {
const modalRef: NgbModalRef = this.modalService.open(
UpdateEditorComponent, { animation: false }
);

//await modalRef.result;
return true;
} catch (_) {
return false;
} finally {
this.menus.setContext(MenuContext.DEFAULT);
}


//router.navigate(["settings"])
}
Copy link
Member

Choose a reason for hiding this comment

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

@tkleinke würdest du das hier lassen oder lieber irgendwo anders einordnen? Die Funktion wird einmal direkt von der AppComponent getriggered (beim Start, wenn username==anonymous, aber auch wenn man später oben in der Navbar auf das User Icon bzw. die Initialen klickt.

Copy link
Member

Choose a reason for hiding this comment

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

Da es ja auch aus der TaskbarEditorComponent heraus aufgerufen wird, sollte es in einen Service, der von beiden Komponenten aufgerufen wird. Ich würde sagen, die Funktion kann in den ProjectModalLauncher, daraus wird auch das Synchronisations-Modal aufgerufen. Evtl. sollte da aber auch eher in die Settings weitergeleitet werden, ansonsten gibt es zwei verschiedene Stellen, an der die gleiche Einstellung vorgenommen werden kann.

@@ -0,0 +1,35 @@
<div class="modal-header">
<!-- TODO: Anpassen -->
<h5 i18n="@@settings.userName">Name des Bearbeiters/der Bearbeiterin</h5>
Copy link
Member

Choose a reason for hiding this comment

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

@tkleinke Translation keys übernommen aus den normalen Settings, deswegen einfach zu übernehmen.

Copy link
Member

Choose a reason for hiding this comment

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

Ja, ich seh da kein Problem, kann so bleiben.

</div>

<div class="modal-body">
<div i18n="@@settings.userName.info">
Copy link
Member

Choose a reason for hiding this comment

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

s.o.

@tkleinke tkleinke merged commit e028a44 into master Dec 12, 2023
4 of 10 checks passed
@dersmon dersmon deleted the editor_name_prompt branch January 17, 2024 11:56
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.

3 participants