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

[ME]: Filter out place keywords in keyword section #987

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

Angi-Kinas
Copy link
Collaborator

Description

This PR fixes the issue that place keywords were shown twice in the editor. Once in the spatial extent section and once on the third page in the keyword section.
Now the keywords are visually separated.

Note:
It is still important to save ALL keywords in the record in the "keywords" property.

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

keyword section of metadata editor,
add e2e test for place keywords and other keywords
Copy link
Contributor

github-actions bot commented Sep 9, 2024

Affected libs: feature-editor,
Affected apps: metadata-editor, datafeeder, demo,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@coveralls
Copy link

coveralls commented Sep 9, 2024

Coverage Status

coverage: 82.082% (+0.6%) from 81.448%
when pulling 95d514b on ME-filter-out-place-kw
into 8b395cf on main.

Copy link
Contributor

github-actions bot commented Sep 9, 2024

📷 Screenshots are here!

Copy link
Collaborator

@LHBruneton-C2C LHBruneton-C2C left a comment

Choose a reason for hiding this comment

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

Looking and working great, except the national / regional coverage keyword is displayed in the generic keywords components, and I think it should not be displayed, same as place keywords. I think you can recognize those specifically from the form fields config.

keywords list in the form (visual), but keep all
keywords in the record.
@@ -175,16 +175,27 @@ export class FormFieldSpatialExtentComponent {
...(thesaurus && { thesaurus }),
} as Keyword)
)
const notPlaceKeywords = await firstValueFrom(

const notPlaceKwAndSpatialScopeKw = await firstValueFrom(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, I couldn't find a better name...

@Angi-Kinas
Copy link
Collaborator Author

@LHBruneton-C2C I filtered out the national / regional keyword from the keywords list on the editor form (visually), but kept them in the record.
I hope this is how it is supposed to be for the user. It doesn't make the code simpler though 😅

Copy link
Collaborator

@LHBruneton-C2C LHBruneton-C2C left a comment

Choose a reason for hiding this comment

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

Maybe not simpler, but that's the expected behavior, thanks!

@Angi-Kinas Angi-Kinas merged commit dcc96ff into main Sep 11, 2024
14 checks passed
@Angi-Kinas Angi-Kinas deleted the ME-filter-out-place-kw branch September 11, 2024 15:46
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