-
Notifications
You must be signed in to change notification settings - Fork 163
feat(Network): Description field feature #978
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #978 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 151 151
Lines 5500 5509 +9
Branches 1105 1066 -39
=========================================
+ Hits 5500 5509 +9 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for implementing this. Overall everything looks great. I just have a few small suggestions for improvement.
- since there are now multiple fields, it would be good to have placeholders on them so it's clear what each field if for.
- I found an existing bug while testing this. If I don't enter a name, the error message is not displayed properly because the lang key doesn't exist in the
en-US.json
file. Would you mind fixing this here in this PR?
- I feel like the description is taking up extra space and doesn't really need to be displayed at all times. I like the tooltip on the main Networks screen. Can you use that info icon on this screen as well?
Hi @jamaljsr Thanks for your review. I'll look into the bug you mentioned and revert to the tooltip. |
20b5949
to
877517b
Compare
Hi @jamaljsr Updated the PR with the bug fix and your suggestion. Kindly review. Thanks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. This looks great now!
Closes #836
Replaces #848
Description
This feature adds a new optional field for the description of each network along with the network name.
Steps to Test
Screenshots
Home view
NetworkView