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.

Sign up for GitHub

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

Jump to bottom

Feature/2066/remove none option from edge metric chooser #2670

Merged
merged 30 commits into from Feb 21, 2022
Merged

Feature/2066/remove none option from edge metric chooser #2670

merged 30 commits into from Feb 21, 2022

Conversation

Hall-Ma
Copy link
Contributor

@Hall-Ma Hall-Ma commented Feb 18, 2022 •
edited

Refactor edge metric panel

Closes: #2066
Closes: #1542

Description

  • When a loaded file hasn't edge metrics then the edge metric panel is hidden.
  • Now you can enable or disable the chosen edge metric (when the file has some).
  • 'None' option is removed from the edge metric panel.
  • When a file has edge metrics then the first edge metric is preselected.

Screenshots or gifs

image

cschild and others added 20 commits February 10, 2022 10:33
@cschild
Remove none from the edge metric list #2066
1922a74
@cschild
Add disable edge metric checkbox #2066
6fbd704
@cschild
Migrate edge metric toggle checkbox #2066
589220d
@cschild
fix edgeMetricToggler property name in appSettings
b29932d
@cschild
Fix disable edge metric when checkbox is checked #2066
11399c6
@cschild
Change checkbox style #2066
0b72647
@cschild
Add edgeMetricToggler to settings-names to reset #2066
41c5266
@Hall-Ma
Add tests for toggling edge metric
e596102 
#2066
@Hall-Ma
Merge branch 'main' into feature/2066/remove-nonve-option-from-edge-m…
cb90bd7 
…etric-chooser
@Hall-Ma
Handle undefined nodes and edges
adbb4bf 
#2066
@Hall-Ma
Add onStoreChanged subscriber for tracking edge metric toggle (workar…
6d46c47 
…ound for communication between AngularJS and Angular)

#2066
@Hall-Ma
Implement showing edge metric panel when file has edge metrics
3150a68 
#2066
@Hall-Ma
Adjust edge metric toggler
f9f846e 
#2066
test: add a test
4f4b02f 
ref #2066
@Hall-Ma
Add css style to disabled edge metric
2a82b4f 
#2066
@Hall-Ma
Adjust tests
64de5a1 
#2066
@Hall-Ma
Adjust tests
9c4d24b 
#2066
@Hall-Ma
Adjust test
294175b 
#2066
@Hall-Ma
Adjust some ccs style
09fb84d 
#2066
@Hall-Ma
Merge branch 'main' into feature/2066/remove-nonve-option-from-edge-m…
c3c84e5 
…etric-chooser
@Hall-Ma Hall-Ma added this to the Good first issues milestone Feb 18, 2022
@Hall-Ma Hall-Ma linked an issue Feb 18, 2022 that may be closed by this pull request
Remove None option from edge metric chooser #2066
Closed
@ce-bo
Copy link
Collaborator

ce-bo commented Feb 19, 2022

Would you please adjust the PR description @cschild / @Hall-Ma. It will help others later to understand what was done.

ce-bo
ce-bo approved these changes Feb 19, 2022
View reviewed changes
Copy link
Collaborator

@ce-bo ce-bo left a comment

Choose a reason for hiding this comment

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

I added some minor thoughts about the naming. However, the code is LGTM!

ce-bo
ce-bo reviewed Feb 19, 2022
View reviewed changes
Copy link
Collaborator

@ce-bo ce-bo left a comment •
edited

Choose a reason for hiding this comment

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

Would it be possible to enable the edge metric if the users changes the selected metric?
For example

  1. avgCommits is pre selected
  2. User disables edge metric
  3. Later, he selects pairingRate for the edge metric
  4. Edge metrics should now be activated atomatically.

@Hall-Ma
Copy link
Contributor Author

Hall-Ma commented Feb 19, 2022

Would it be possible to enable the edge metric if the users changes the selected metric? For example

  1. avgCommits is pre selected
  2. User disables edge metric
  3. Later, he selects pairingRate for the edge metric
  4. Edge metrics should now be activated automatically.

You're totally right. It seems to be the better behavior. I have already changed it.

@sonarcloud
Copy link

sonarcloud bot commented Feb 21, 2022

[CodeCharta Analysis] 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

@sonarcloud
Copy link

sonarcloud bot commented Feb 21, 2022

[CodeCharta Visualization] 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 2 Code Smells

97.1% 97.1% Coverage
0.0% 0.0% Duplication

@IhsenBouallegue IhsenBouallegue merged commit 4ad684e into main Feb 21, 2022
@IhsenBouallegue IhsenBouallegue deleted the feature/2066/remove-none-option-from-edge-metric-chooser branch February 21, 2022 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ce-bo ce-bo ce-bo approved these changes

@Christian-Eberhard Christian-Eberhard Christian-Eberhard left review comments

@RomanenkoVladimir RomanenkoVladimir RomanenkoVladimir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Good first issues
Development

Successfully merging this pull request may close these issues.

Remove None option from edge metric chooser Edge metric is only shown if there are activated edges
6 participants
@Hall-Ma @ce-bo @RomanenkoVladimir @Christian-Eberhard @IhsenBouallegue @cschild