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

Implement search in Details views #4318

Merged
merged 34 commits into from
Jul 9, 2024
Merged

Implement search in Details views #4318

merged 34 commits into from
Jul 9, 2024

Conversation

RobertJoonas
Copy link
Contributor

Changes

  • Refactor the structure of a metric on the frontend - use a Metric class with special creator functions.
  • Create a higher-order component for keeping the query context (such as the query object itself and additional information such as revenueAvailable, importedDataInView, lastLoadTimestamp)
  • Create a new BreakdownModal component that includes the search functionality, and make all current "Details" view modals to use it.
    • Note: some API endpoints (goals, countries, regions, cities) do not support a contains filter. This PR will start using the new BreakdownModal in those reports too, but the search is explicitly disabled via enableSearch={false} prop.

Tests

  • This PR does not require tests

Changelog

  • Entry has been added to changelog

Documentation

  • Docs have been updated

Dark mode

  • The UI has been tested both in dark and light mode

@RobertJoonas RobertJoonas marked this pull request as draft July 8, 2024 08:19
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 8, 2024

Preview environment👷🏼‍♀️🏗️
PR-4318

RobertJoonas added 25 commits July 8, 2024 12:23
@RobertJoonas
Create a new BreakdownModal component and use it for Entry Pages
be9a185
@RobertJoonas
Add search functionality into the new component
d0094a7
@RobertJoonas
Adjust FilterLink component and use it in BreakdownModal
caac244
@RobertJoonas
pass addSearchFilter fn through props
3a6e9ec
@RobertJoonas
pass fn props as useCallback
8726011
@RobertJoonas
add a function doc to BreakdownModal
787e9fb
@RobertJoonas
refactor: create a Metric class
25df55d
@RobertJoonas
Fixup: use Metric class for defining BreakdownModal metrics
b41ba07
@RobertJoonas
keep revenueAvailable state in the Dashboard component
84533e9
@RobertJoonas
move query context into a higher-order component
8e686ea
@RobertJoonas
fix react key error in BreakdownModal
6c7d6ca
@RobertJoonas
use BreakdownModal in PropsModal
1f00120
@RobertJoonas
adjust EntryPagesModal to use query context
aa7d19a
@RobertJoonas
fix variable name typo
a918716
@RobertJoonas
fixup: BreakdownModal function doc
b5ec45f
@RobertJoonas
use BreakdownModal in SourcesModal
cd72fe0
@RobertJoonas
use Breakdown modal in ReferrerDrilldownModal
413fdd0
@RobertJoonas
use BreakdownModal in PagesModal
280634a
@RobertJoonas
use BreakdownModal in ExitPagesModal
84a81be
@RobertJoonas
replace ModalTable with LocationsModal and use BreakdownModal in it
7bd867e
@RobertJoonas
use BreakdownModal in Conversions
4d61b61
@RobertJoonas
make sure next pages are loaded with 'detailed: true'
e599077
@RobertJoonas
replace loading spinner logic in BreakdownModal
db4903a
@RobertJoonas
fix two flaky tests
3029526
@RobertJoonas
unfocus search input element on Escape keyup event
37326b9
@RobertJoonas RobertJoonas marked this pull request as ready for review July 8, 2024 10:57
@RobertJoonas RobertJoonas requested a review from ukutaht July 8, 2024 10:57
@RobertJoonas RobertJoonas requested a review from ukutaht July 9, 2024 10:18
macobo
macobo reviewed Jul 9, 2024
View reviewed changes
assets/js/dashboard/stats/reports/list.js
<FilterLink
query={props.query}
filterInfo={props.getFilterFor(listItem)}
onClick={props.onClick || noop}
Copy link
Contributor

@macobo macobo Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Why is the || noop needed? Can't the FilterLink component handle undefined as react components do usually?

@RobertJoonas RobertJoonas merged commit 7d0321f into master Jul 9, 2024
10 checks passed
@RobertJoonas RobertJoonas deleted the modal-search branch July 9, 2024 12:01
@RobertJoonas RobertJoonas mentioned this pull request Jul 9, 2024
Merged
4 tasks
@aerosol
Copy link
Member

aerosol commented Jul 9, 2024

Well done, this is really cool. WYT about treating the search input with autofocus?

@RobertJoonas RobertJoonas mentioned this pull request Jul 10, 2024
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@macobo macobo macobo left review comments

@apata apata apata left review comments

@ukutaht ukutaht ukutaht approved these changes

Assignees
No one assigned
Labels
preview
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@RobertJoonas @aerosol @macobo @ukutaht @apata