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

feat(project): proof of new project workflow concept (DEV-985) #760

Merged
merged 14 commits into from Jun 10, 2022

Conversation

kilchenmann
Copy link
Contributor

@kilchenmann kilchenmann commented Jun 7, 2022

resolves DEV-985

@kilchenmann kilchenmann self-assigned this Jun 7, 2022
@kilchenmann kilchenmann added the enhancement New feature or request label Jun 7, 2022
@kilchenmann kilchenmann marked this pull request as draft Jun 7, 2022
@kilchenmann kilchenmann changed the title feat(project): Proof of new project workflow concept (DEV-985) feat(project): proof of new project workflow concept (DEV-985) Jun 8, 2022
@kilchenmann kilchenmann marked this pull request as ready for review Jun 9, 2022
@kilchenmann kilchenmann requested a review from mdelez as a code owner Jun 9, 2022
Copy link
Collaborator

@mdelez mdelez left a comment

I'm really liking the new look of it all! Just a few remarks and change requests.


<!-- add new resource instance if instance id is called "add" -->
<div class="single-instance-form" *ngIf="instanceId && instanceId === 'add'">
<h3>Create new {{classId}} instance</h3>
Copy link
Collaborator

@mdelez mdelez Jun 9, 2022

Choose a reason for hiding this comment

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

This should be done a bit nicer so that only the class name is used in the sentence instead of the entire class id string

Copy link
Contributor Author

@kilchenmann kilchenmann Jun 10, 2022

Choose a reason for hiding this comment

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

Sure thing. This will be done in the next iteration. Because I have only the IRI here and I have to make an API request first etc.

if (this.instanceId) {
// single instance

if (this.instanceId === 'add') {
Copy link
Collaborator

@mdelez mdelez Jun 9, 2022

Choose a reason for hiding this comment

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

Is there a better way to make this logic for showing the create new res class instance form? Having empty if statements that appear to do nothing at a glance is a bit confusing.

Copy link
Contributor Author

@kilchenmann kilchenmann Jun 10, 2022

Choose a reason for hiding this comment

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

In case of this.instanceId we have two options: in case of "add" the create-resource-instance-form is displayed. In case of "real" resource instance IRI/ID the single instance will be displayed. I thought to implement this view in the next iteration.
Maybe there's a third option called "/conf" where a project admin can edit the resource class itself.

I see, the chosen name "instanceId" is really confusing. I'll think about a better setup and will replace in the next branch.

src/app/project/project.component.ts Outdated Show resolved Hide resolved
src/app/workspace/results/results.component.ts Outdated Show resolved Hide resolved
src/assets/style/_layout.scss Outdated Show resolved Hide resolved
@kilchenmann kilchenmann requested a review from mdelez Jun 10, 2022
mdelez
mdelez approved these changes Jun 10, 2022
@kilchenmann kilchenmann merged commit 2391f2a into main Jun 10, 2022
13 checks passed
@kilchenmann kilchenmann deleted the wip/new-project-concept branch Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants