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

Add option to rename facts #2811

Merged
merged 7 commits into from
Sep 12, 2023
Merged

Add option to rename facts #2811

merged 7 commits into from
Sep 12, 2023

Conversation

JamieScottC
Copy link
Contributor

@JamieScottC JamieScottC commented Aug 29, 2023

Description

  • This adds an input field to a fact source card which allows the ability to rename facts name/trait
  • Also removes any weird behavior when deleting values from a fact. Previously values wouldn't be deleted or they would be, but it wouldn't be reflected on the UI.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Bug fix

How Has This Been Tested?

I have tried editing multiple fact names and then refreshed to make sure the changes are saved

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code

@JamieScottC JamieScottC marked this pull request as ready for review August 30, 2023 00:07
@JamieScottC JamieScottC requested a review from clenk August 30, 2023 00:07
@sonarcloud
Copy link

sonarcloud bot commented Aug 30, 2023

Please retry analysis of this Pull-Request directly on SonarCloud.

@JamieScottC
Copy link
Contributor Author

Also just pushed another commit to hopefully fix any weird behavior when deleting values from a fact.

@JamieScottC JamieScottC requested a review from morinjmit August 31, 2023 15:02
@morinjmit
Copy link
Contributor

Is it possible to just make the name field thats already there editable when a user clicks on a fact? So like instead of adding another input below the fact name to change can you make the top one able to be edited? If not maybe hide the top one when the edit window is open just so the fact name isn't showing up twice which could be a little confusing or less straight forward to the user.

Other than that looks good to me

@JamieScottC
Copy link
Contributor Author

Is it possible to just make the name field thats already there editable when a user clicks on a fact? So like instead of adding another input below the fact name to change can you make the top one able to be edited? If not maybe hide the top one when the edit window is open just so the fact name isn't showing up twice which could be a little confusing or less straight forward to the user.

Other than that looks good to me

Good point! Just pushed that change

@elegantmoose elegantmoose self-requested a review September 6, 2023 20:58
@JamieScottC JamieScottC removed the request for review from clenk September 6, 2023 21:03
morinjmit
morinjmit previously approved these changes Sep 7, 2023
Copy link
Contributor

@morinjmit morinjmit left a comment

Choose a reason for hiding this comment

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

Looks good now!

clenk
clenk previously approved these changes Sep 7, 2023
Copy link
Contributor

@clenk clenk left a comment

Choose a reason for hiding this comment

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

LGTM

@JamieScottC JamieScottC dismissed stale reviews from morinjmit and clenk via fc30b0b September 11, 2023 16:37
@JamieScottC
Copy link
Contributor Author

I just committed another fix for a bug that @elegantmoose noticed. Hopefully one of the last🤞!

@elegantmoose elegantmoose merged commit e7b2d8c into master Sep 12, 2023
5 of 9 checks passed
@elegantmoose elegantmoose deleted the feature/rename-facts branch September 12, 2023 01:01
@sonarcloud
Copy link

sonarcloud bot commented Sep 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

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.

4 participants