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.

Sign up for GitHub

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

Jump to bottom

Added language icons modal on Search page #768

Merged
merged 10 commits into from
Feb 2, 2024
Merged

Added language icons modal on Search page #768

merged 10 commits into from
Feb 2, 2024

Conversation

Anahkiasen
Copy link
Contributor

@Anahkiasen Anahkiasen commented Jan 25, 2024 •
edited
Loading

Added

  • Added Editorconfig file to ensure consistent code style
  • Added language icons modal on Search page

Fixed

  • Fixed some imagemin dependencies not being declared at the top level
Screenflick.Movie.74.mp4

Requires cultuurnet/udb3-angular-app#211

Ticket: https://jira.publiq.be/browse/III-5982

@Anahkiasen Anahkiasen self-assigned this Jan 25, 2024
@Anahkiasen Anahkiasen changed the title Fix some transient dependencies not being declared Added language icons modal on Search page Jan 25, 2024
@Anahkiasen Anahkiasen marked this pull request as draft January 25, 2024 13:20
@Anahkiasen
Copy link
Contributor Author

Forgot to conditionally hide depending on app language

@Anahkiasen Anahkiasen marked this pull request as ready for review January 26, 2024 11:27
brampauwelyn
brampauwelyn approved these changes Feb 1, 2024
View reviewed changes
src/entry/labelling/offer-languages-modal.html
@@ -0,0 +1,23 @@
<div class="modal-body offer-languages-modal">
<h2>Taaliconen toevoegen</h2>
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably also be moved to the translations file?

src/entry/labelling/offer-languages-modal.html
<div class="modal-body offer-languages-modal">
<h2>Taaliconen toevoegen</h2>
<div class="alert alert-info" role="alert">
Aan de hand van taaliconen geef je aan hoeveel kennis van het Nederlands vereist is om aan je aanbod te kunnen
Copy link
Contributor

Choose a reason for hiding this comment

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

Could probably also be moved to the translations file?

src/search/ui/search.html Outdated Show resolved Hide resolved
@Anahkiasen
Copy link
Contributor Author

@brampauwelyn Sorry for the translations they were kept intentionally hardcoded as per the ticket since this is a dutch-specific feature:

Language icons are only applicable in the Flemish region of Belgium, so it doesn't make sense to provide this functionality for users using the FR or DE interface, or to provide FR or DE copy for it.

@Anahkiasen Anahkiasen merged commit 061be0f into master Feb 2, 2024
1 check passed
@Anahkiasen Anahkiasen deleted the feature/III-5982 branch February 2, 2024 11:12
@brampauwelyn
Copy link
Contributor

@brampauwelyn Sorry for the translations they were kept intentionally hardcoded as per the ticket since this is a dutch-specific feature:

Language icons are only applicable in the Flemish region of Belgium, so it doesn't make sense to provide this functionality for users using the FR or DE interface, or to provide FR or DE copy for it.

Ah yes true 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simon-debruijn simon-debruijn simon-debruijn approved these changes

@brampauwelyn brampauwelyn brampauwelyn approved these changes

Assignees

@Anahkiasen Anahkiasen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Anahkiasen @brampauwelyn @simon-debruijn