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

Feature/Routing-Forms/Reporting #5375

Merged
merged 23 commits into from Nov 11, 2022
Merged

Feature/Routing-Forms/Reporting #5375

merged 23 commits into from Nov 11, 2022

Conversation

hariombalhara
Copy link
Member

@hariombalhara hariombalhara commented Nov 4, 2022

What does this PR do?

Closes #5440

Demo

Screenshot 2022-11-09 at 9 13 48 PM

Environment: Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How should this be tested?

  • See loom

Note: You would see 2 unit tests check below. One is passing and the other is failing. This is because I have a fix for unit tests workflow in my PR which is passing and main one is failing

@vercel
Copy link

vercel bot commented Nov 4, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ✅ Ready (Inspect) Visit Preview Nov 11, 2022 at 9:58AM (UTC)

@what-the-diff
Copy link

what-the-diff bot commented Nov 4, 2022

  • Added a new page called reporting
  • Updated the routing config to include this new route
  • Created an initial query builder configuration for the reporter row in react-awesome-query-builder/config/config file and updated it with fields from form's schema (formId, fieldName)
  • Implemented ReporterRow component which renders QueryBuilder along with Result table that shows responses based on jsonLogicQuery generated by QueryBuilder

@hariombalhara hariombalhara self-assigned this Nov 4, 2022
@hariombalhara
Copy link
Member Author

@harshsinghatz this isn't ready for review yet. It's in draft

@CarinaWolli
Copy link
Member

Awesome feature, works great 🙌🏻

I only have a few design suggestions:

  • There is more space on the right side of table (I think there is a mr-2 somewhere)
    Screenshot 2022-11-10 at 10 11 23

  • Pagination would probably make sense to have as there will be a lot of data over time (maybe this can also wait and be added later)

  • Should we make the corners of the table rounded to fit our design?

Copy link
Member

@PeerRich PeerRich left a comment

Choose a reason for hiding this comment

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

the gray headline looks a bit dark, a lighter dark is better

also "Add Query" should be "Add Filter", no?

@hariombalhara
Copy link
Member Author

hariombalhara commented Nov 11, 2022

@PeerRich

  • Changed Add Query -> Add Filter. It would resonate with more people.
  • Made header bg-gray-200

Screenshot 2022-11-11 at 11 57 07 AM

@CarinaWolli

There is more space on the right side of table (I think there is a mr-2 somewhere)

This is because the table is left aligned. The space at right would vary

Pagination would probably make sense to have as there will be a lot of data over time (maybe this can also wait and be added later)

Yeah I thought the same. The user can always filter the data to limit what loads on the page

Should we make the corners of the table rounded to fit our design

Wanted to do it but it wasn't straightforward. Tables are tricky. Implemented it now.

@kodiakhq kodiakhq bot merged commit bb78154 into main Nov 11, 2022
@kodiakhq kodiakhq bot deleted the feat/routing-forms/reporting branch November 11, 2022 09:57
@PeerRich PeerRich added the core area: core, team members only label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge ♻️ autoupdate tells kodiak to keep this branch up-to-date core area: core, team members only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CAL-337] Routing Forms - Reporting
4 participants