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(advanced search): try to reuse resource and property selection (DSP-1587) #290

Merged
merged 23 commits into from May 6, 2021

Conversation

tobiasschweizer
Copy link
Contributor

@tobiasschweizer tobiasschweizer commented May 3, 2021

resolves DSP-1587

I decided to do this task in two steps:

  • move resource class and property selection to a reusable component (this PR)
  • use this new component in main form and for linking property specification (upcoming PR)

@tobiasschweizer tobiasschweizer added the enhancement New feature or request label May 3, 2021
@tobiasschweizer tobiasschweizer self-assigned this May 3, 2021
@tobiasschweizer tobiasschweizer marked this pull request as draft May 3, 2021 11:57
@tobiasschweizer tobiasschweizer marked this pull request as ready for review May 5, 2021 08:54
@tobiasschweizer tobiasschweizer requested review from mdelez, kilchenmann and subotic and removed request for mdelez May 5, 2021 08:56

const input = await loader.getHarness(MatInputHarness);

await input.setValue('1');

expect(await submitButton.isDisabled()).toBe(false);
const submitButton5 = await page.getAdvancedSearchSubmitButton(loader);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mdelez I had to add this workaround because the existing instance always returned the old value for the state of the disabled prop.

I believe that we had a similar problem once but there the element was recreated in the DOM which is not the case here. Fishing in the dark here ...

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah from what I can remember, it won't update automatically if the value changes but I think you can reuse the same variable, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but I think you can reuse the same variable, no?

Yes, good point. I think. I could even avoid using a variable in the first place, see 7e21377

This failed locally, let's see how it behaves here.

Copy link
Contributor

@mdelez mdelez left a comment

Choose a reason for hiding this comment

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

It works quite well on my end! I did notice that when you select either "is equal to" or "is not equal to" for a boolean value, you get two checkboxes. The second checkbox doesn't affect the search in any way. It's just hanging out with its buddy :)

Screen Shot 2021-05-06 at 09 36 44


const input = await loader.getHarness(MatInputHarness);

await input.setValue('1');

expect(await submitButton.isDisabled()).toBe(false);
const submitButton5 = await page.getAdvancedSearchSubmitButton(loader);
Copy link
Contributor

Choose a reason for hiding this comment

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

yeah from what I can remember, it won't update automatically if the value changes but I think you can reuse the same variable, no?

@tobiasschweizer
Copy link
Contributor Author

It works quite well on my end! I did notice that when you select either "is equal to" or "is not equal to" for a boolean value, you get two checkboxes. The second checkbox doesn't affect the search in any way. It's just hanging out with its buddy :)

Screen Shot 2021-05-06 at 09 36 44

Right :) One is for the Boolean value and one is for the sort criterion. There is even an old task for this: https://dasch.myjetbrains.com/youtrack/issue/DSP-354

@mdelez
Copy link
Contributor

mdelez commented May 6, 2021

It works quite well on my end! I did notice that when you select either "is equal to" or "is not equal to" for a boolean value, you get two checkboxes. The second checkbox doesn't affect the search in any way. It's just hanging out with its buddy :)
Screen Shot 2021-05-06 at 09 36 44

Right :) One is for the Boolean value and one is for the sort criterion. There is even an old task for this: https://dasch.myjetbrains.com/youtrack/issue/DSP-354

ah, I missed that :)

@tobiasschweizer
Copy link
Contributor Author

It works quite well on my end! I did notice that when you select either "is equal to" or "is not equal to" for a boolean value, you get two checkboxes. The second checkbox doesn't affect the search in any way. It's just hanging out with its buddy :)
Screen Shot 2021-05-06 at 09 36 44

Right :) One is for the Boolean value and one is for the sort criterion. There is even an old task for this: https://dasch.myjetbrains.com/youtrack/issue/DSP-354

ah, I missed that :)

maybe I should actually do this task :-)

@tobiasschweizer tobiasschweizer merged commit 523af24 into main May 6, 2021
@tobiasschweizer tobiasschweizer deleted the wip/dsp-1587-search-linked-res branch May 6, 2021 09:24
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