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

fix(ui): assistance for arrays data in constraints #2889

Merged
merged 5 commits into from
Mar 23, 2024

Conversation

erka
Copy link
Collaborator

@erka erka commented Mar 22, 2024

closes #2885

@erka
Copy link
Collaborator Author

erka commented Mar 22, 2024

@markphelps it isn't ready but I would like to get you feedback and thoughts.

@erka
Copy link
Collaborator Author

erka commented Mar 22, 2024

Screen Shot

Copy link

codecov bot commented Mar 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.14%. Comparing base (f997fb9) to head (4010728).
Report is 163 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2889      +/-   ##
==========================================
+ Coverage   70.78%   72.14%   +1.36%     
==========================================
  Files          91       92       +1     
  Lines        8729     7102    -1627     
==========================================
- Hits         6179     5124    -1055     
+ Misses       2165     1594     -571     
+ Partials      385      384       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

looks great so far @erka as usual!! Thank you for taking this on!

I know you said it wasn't ready, but I tested it and works great for the most part. Just a couple minor things I noticed:

CleanShot 2024-03-22 at 16 34 07@2x

we may want to make sure we strip certain characters like [ ] from the strings, as above caused an error:

CleanShot 2024-03-22 at 16 38 19

  1. we probably want to truncate the values in the UI

CleanShot 2024-03-22 at 16 34 47@2x

  1. we may want to play around with the colors a bit to try something lighter for the pills? wdyt?

@erka erka marked this pull request as ready for review March 22, 2024 22:25
@erka erka requested a review from a team as a code owner March 22, 2024 22:25
@erka
Copy link
Collaborator Author

erka commented Mar 22, 2024

@markphelps I've addressed all of them except the first one. I don't see any issues with special characters, maybe console.log make you think it's broken.

Copy link
Collaborator

@markphelps markphelps left a comment

Choose a reason for hiding this comment

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

looks and works amazing @erka !!!

@markphelps markphelps added the automerge Used by Kodiak bot to automerge PRs label Mar 23, 2024
@kodiakhq kodiakhq bot merged commit bbf0a91 into flipt-io:main Mar 23, 2024
27 checks passed
@markphelps
Copy link
Collaborator

I tested it for backward compatibility, from the current release of Flipt which only renders the raw array ['values', 'foo'] and it renders them correctly when built with this change

@erka erka deleted the ui-segment-array-input branch March 23, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Used by Kodiak bot to automerge PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI assistance for arrays data in constraints
2 participants