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

New manual add design #2530

Merged
merged 22 commits into from
Feb 24, 2023
Merged

New manual add design #2530

merged 22 commits into from
Feb 24, 2023

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Feb 7, 2023

🚨 do not merge as this is only half of a feature 🚨

Closes #2434

Code Changes

  • Remove the old manual system flow (cleans up a lot of the config wizard slice)
  • New SystemCatalog component which renders the connectors list (for now). /add-systems/new
  • Update search bar to include a clear button
  • Fixes bug where editing a system via /system/configure had "back" buttons which did nothing
  • Update cypress tests

Steps to Confirm

  • Visit /add-systems and click "Add a system"
  • Click around the list of system types. When you click into them, they should render their system type and logo
  • Create a system without a system type. It should say "Describe your new system" with the Ethyca logo as a placeholder
  • Test out search
  • You may also want to test out editing a system ("View system" --> click on ellipsis for more actions on a system card --> "Edit"). This had some weird behavior on main which I updated. Note that this page looks different in nav v1 and nav v2

Pre-Merge Checklist

Description Of Changes

Screen.Recording.2023-02-07.at.3.22.44.PM.mov

Search when there are no results

Screen.Recording.2023-02-07.at.3.24.54.PM.mov

@codecov
Copy link

codecov bot commented Feb 7, 2023

Codecov Report

Base: 86.46% // Head: 86.46% // No change to project coverage 👍

Coverage data is based on head (9c66f38) compared to base (72b5dfe).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2530   +/-   ##
=======================================
  Coverage   86.46%   86.46%           
=======================================
  Files         289      289           
  Lines       15984    15984           
  Branches     2026     2026           
=======================================
  Hits        13821    13821           
  Misses       1779     1779           
  Partials      384      384           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@allisonking allisonking changed the base branch from main to aking/2434/add-system-redesign February 7, 2023 19:46
@allisonking allisonking marked this pull request as ready for review February 7, 2023 20:28
Base automatically changed from aking/2434/add-system-redesign to main February 7, 2023 20:40
@allisonking allisonking force-pushed the aking/2434/new-manual-add-design branch from ac48da7 to b662e9d Compare February 8, 2023 16:45
@allisonking allisonking force-pushed the aking/2434/new-manual-add-design branch from b662e9d to 44b7592 Compare February 8, 2023 16:47
Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

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

Everything is working well! I think I may have found a small css issue with the clear button on the search bar. It looks like the is overlaying on top of the inputs border.

Screenshot 2023-02-09 at 11 09 22

Pretty minor though. Are you seeing this locally?

@seanpreston seanpreston added the do not merge Please don't merge yet, bad things will happen if you do label Feb 9, 2023
@allisonking
Copy link
Contributor Author

Everything is working well! I think I may have found a small css issue with the clear button on the search bar. It looks like the is overlaying on top of the inputs border.

Screenshot 2023-02-09 at 11 09 22

Pretty minor though. Are you seeing this locally?

You've got sharp eyes! 🦅 👁️

looks like I can adjust the width and height by 95% to avoid the focus state overlapping:

focused:
image

unfocused:
image

@cypress
Copy link

cypress bot commented Feb 14, 2023

Passing run #387 ↗︎

0 3 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge 9c66f38 into 72b5dfe...
Project: fides Commit: fac180bb6b ℹ️
Status: Passed Duration: 00:38 💡
Started: Feb 24, 2023 3:57 PM Ended: Feb 24, 2023 3:58 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Contributor

@TheAndrewJackson TheAndrewJackson left a comment

Choose a reason for hiding this comment

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

Thanks for fixing that styling issue! 🚀

@allisonking allisonking removed the do not merge Please don't merge yet, bad things will happen if you do label Feb 24, 2023
@seanpreston seanpreston self-requested a review February 24, 2023 16:13
@allisonking allisonking merged commit 415f554 into main Feb 24, 2023
@allisonking allisonking deleted the aking/2434/new-manual-add-design branch February 24, 2023 18:09
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.

Replace "add a system manually" route with a page to select from connectors
3 participants