-
Notifications
You must be signed in to change notification settings - Fork 5
Feat/217 add new registries #131
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
Conversation
@@ -34,4 +35,5 @@ export const STATES = [ | |||
WikiState, | |||
MeetingsState, | |||
RegistrationsState, | |||
RegistriesState, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it from here and use it in registries routes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this.
|
||
@Component({ | ||
selector: 'osf-registries', | ||
imports: [RouterModule], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use RouterOutlet
instead of RouterModule
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add all exports.
@@ -0,0 +1,11 @@ | |||
import { Provider } from '../models'; | |||
import { ProvidersResponseJsonApi } from '../models/providers-json-api.model'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify it.
@@ -0,0 +1,11 @@ | |||
import { Project } from '../models'; | |||
import { ProjectsResponseJsonApi } from '../models/projects-json-api.model'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplify it.
…to feat/217-add-new-registries
…to feat/217-add-new-registries
…to feat/217-add-new-registries
…to feat/217-add-new-registries
…nScience/angular-osf into feat/217-add-new-registries
…to feat/217-add-new-registries
…to feat/217-add-new-registries
…to feat/217-add-new-registries
…nScience/angular-osf into feat/217-add-new-registries
…to feat/217-add-new-registries
…to feat/217-add-new-registries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add also here subject-service.model
export.
patchState({ | ||
providers: { | ||
data: providers, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty line
|
||
export class PageSchemaMapper { | ||
static fromSchemaBlocksResponse(response: SchemaBlocksResponseJsonApi): PageSchema[] { | ||
console.log('PageSchemaMapper.fromSchemaBlocksResponse', response); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove console.log
.
|
||
export const defaultSteps: StepOption[] = [ | ||
{ | ||
label: 'Metadata', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about translations?
projectId: this.fromProject ? (project ?? undefined) : undefined, | ||
}) | ||
.subscribe(() => { | ||
this.toastService.showSuccess('Draft created successfully'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add translation.
<h3 class="mb-2">Upload File</h3> | ||
<p class="mb-1">You may attach up to 5 file(s) to this question. Files cannot total over 5GB in size.</p> | ||
<p> | ||
Uploaded files will automatically be archived in this registration. They will also be added to a related | ||
project that will be created for this registration. | ||
</p> | ||
|
||
<p>File input is not implemented yet.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Translation
this.route.params.subscribe((params) => { | ||
this.step.set(+params['step'].split('-')[0]); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try to use toSignal
. I think you can add it instead of step = signal(this.route.snapshot.params['step'].split('-')[0]);
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you can see step is already signal, but without subscription, it doesn't reflect on the params changes
} | ||
|
||
stepChange(step: number): void { | ||
// TODO: before navigating, validate the current step |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove it or add correct format: [NM] TODO: ...
.
} | ||
|
||
onFocusOut() { | ||
// TODO: make request to update contributor if changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make it right format [NM] TODO:
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename component files to registries-subjects
No description provided.