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

Uhf 6666 search filters rebased #378

Conversation

sakkelaaksonen-siili
Copy link

UHF-6666: Search filters

Added filters to LinkedEvents React-version

What was done

  • Added filters according to specs
  • Filters rendered conditionally based on paragraph choices
  • Selecting stuff from filters and submitting creates an API call to LinkedEvents and renders results

How to install

  • Get any instance up and running
  • Get this branch: composer require drupal/helfi_platform_config:dev-UHF-6666-search-filters
  • Get the style branch for HDBT: composer require drupal/hdbt:dev-UHF-3128-sote-linkedevents-filtering
  • Run make drush-updb
  • Run make drush-cr

How to test

  • Create or edit a landing- or standard page.
  • Add an Events paragraph. Fill fields.
    • API URL = https://tapahtumat.hel.fi/fi/events?text=jooga
  • Visit the page.
  • Test the filters:
    • Changing filter values shouldn't execute search, only clicking the submit button
    • Test that filters work as expected
  • "Lataa lisää" will be removed. At this point, only 5 first events will be shown, regadless of result count. This will be handled later with Pagination component.

Other PRs

jeremysteerio and others added 30 commits October 12, 2022 14:35
@sakkelaaksonen-siili
Copy link
Author

Sivuhuomiona tuo Feature-branch on väärin nimetty.
Oikea tiketti on https://helsinkisolutionoffice.atlassian.net/browse/UHF-3218

https://helsinkisolutionoffice.atlassian.net/browse/UHF-3128 on jotain ihan muuta.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Contributor

@Arskiainen Arskiainen left a comment

Choose a reason for hiding this comment

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

This seems real nice for the most part. There are a lot of weird formatting issues that should be solved on the project level. Probably need to revisit the linter/prettier setup and enforce some basic stuff.

@@ -1,21 +1,22 @@
{
"name": "helfi-etusivu-news-search",
"version": "0.1.0",
"version": "0.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely more than a patch version increase needed.

},
"scripts": {
"start": "BROWSER=none react-app-rewired start",
"build": "react-app-rewired build",
"build": "react-app-rewired build && mv build/static/js/main.*.js ../assets/js/filter-events.js",
"":"",
Copy link
Contributor

Choose a reason for hiding this comment

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

?


import useOutsideClick from '../hooks/useOutsideClick';

type Props = {
Copy link
Contributor

Choose a reason for hiding this comment

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

CollapsibleProps maybe?

Comment on lines +1 to +6
import Collapsible from '../components/Collapsible';
import { DateInput } from 'hds-react';
import { QueryBuilder } from '../utils/QueryBuilder'
import CheckboxFilter from '../components/CheckboxFilter';
import type DateSelectDateTimes from '../types/DateSelectDateTimes';
import HDS_DATE_FORMAT from '../utils/HDS_DATE_FORMAT';
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem to be in a random order. The linter should probably complain if there is no empty lines between imports and definitions.

const dateHelperText = Drupal.t('Use format D.M.YYYY')
const dateLabel = Drupal.t('Choose a date')

const DateSelect = ({ endDate, endDisabled, disableEnd, queryBuilder, setEndDate, setStartDate, startDate, invalidStartDate = false, invalidEndDate = false }: DateSelectProps) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be prettiered automatically.



const dateHelperText = Drupal.t('Use format D.M.YYYY')
const dateLabel = Drupal.t('Choose a date')
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be the same for start and end. Bad UX.

<DateInput
className='hdbt-search__filter hdbt-search__date-input'
helperText={dateHelperText}
id='start-date'
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these ids unique'd in the component?

Comment on lines +6 to +7
<h3>{Drupal.t('This event list is empty.')}</h3>
<p className='events-list__empty-subtext'>{Drupal.t('No worries though, this city does not run out of things to do.')}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to extract these content strings into one place at some point.

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