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

Improve formulas UI/input #3620

Merged
merged 7 commits into from
Apr 5, 2021
Merged

Improve formulas UI/input #3620

merged 7 commits into from
Apr 5, 2021

Conversation

timgl
Copy link
Collaborator

@timgl timgl commented Mar 10, 2021

Changes

Only allow certain characters to be entered, add letters next to the graph series

image

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@timgl timgl temporarily deployed to posthog-improve-formula-pjcoqt March 10, 2021 15:45 Inactive
@timgl timgl requested a review from paolodamico March 16, 2021 14:09
@timgl timgl marked this pull request as ready for review March 16, 2021 14:09
Copy link
Contributor

@paolodamico paolodamico left a comment

Choose a reason for hiding this comment

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

lgtm! suggesting some UI adjustments. While I do think this is a great improvement, I think the UX of this is still a bit wanting (for a separate PR). I was thinking that maybe we could autocomplete the input and instead of the user typing "A", they would select the element, and we would add more info on each element, something along these lines.

// Only allow typing of allowed characters
value = value
.split('')
.filter((d) => /^[a-zA-Z\ \-\*\^0-9\+\/\(\)]+$/g.test(d))
Copy link
Contributor

Choose a reason for hiding this comment

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

You could even restrict to allow adding only letters for the number of graphs currently being showed.

@paolodamico
Copy link
Contributor

Pushed commit adjusts the styling a bit,

And fixes this jumpiness,

image

@paolodamico
Copy link
Contributor

Let me know if you want me to take another look

@timgl timgl merged commit fbd6844 into master Apr 5, 2021
@timgl timgl deleted the improve-formulas branch April 5, 2021 11:00
@timgl
Copy link
Collaborator Author

timgl commented Apr 5, 2021

@paolodamico I've merged, I checked your changes and they looked good. Worth checking quickly on master, then we can probably start releasing to 10-20% of users?

@paolodamico
Copy link
Contributor

We were a bit stuck on deploys today but all good now. Tested on production and seems to work just fine. Releasing right now to 15% of all real users. Maybe worth opening an issue on the experiments repo to document the experiment?

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.

None yet

2 participants