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

[redesign] Settings view #3631

Merged
merged 14 commits into from
Feb 2, 2022
Merged

[redesign] Settings view #3631

merged 14 commits into from
Feb 2, 2022

Conversation

bgptr
Copy link
Collaborator

@bgptr bgptr commented Jan 2, 2022 •
edited
Loading

Builds upon #3611
Requires decred/pi-ui#413 (merged)
Closes #3613

Notable changes:

  • implements the redesigned Settings view.
  • applies pi-ui Select component & ditch react-select lib
  • removed the save button and implemented the autosave feature. Also added a Settings saved successfully. snack bar.
  • update automated tests

Screenshots:
image
image
image
image

@bgptr bgptr mentioned this pull request Jan 2, 2022
Merged
@bgptr bgptr force-pushed the redesign-settings branch 3 times, most recently from 1f95b52 to 9ea1f6e Compare January 17, 2022 16:51
@alexlyp
Copy link
Member

alexlyp commented Feb 1, 2022

Ready to give this a PR a look. Please rebase. Thanks!

@alexlyp alexlyp added this to the v1.7.1 milestone Feb 2, 2022
@bgptr bgptr force-pushed the redesign-settings branch from 715daf3 to 6191abc Compare February 2, 2022 08:02
@bgptr bgptr mentioned this pull request Feb 2, 2022
Merged
matheusd
matheusd approved these changes Feb 2, 2022
View reviewed changes
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Tested fine.

Nice job getting rid of react-select!

@alexlyp
Copy link
Member

alexlyp commented Feb 2, 2022

Overall looking great.

One thing that stood out is the usage of the tooltips on the Connectivity tab. Is the idea to say that it will require a restart after changing these? Currently doesn't make a ton of sense

@bgptr
Copy link
Collaborator Author

bgptr commented Feb 2, 2022

@alexlyp Actually, I think the tooltips should indicate if a certain parameter was set from cli. The pi-ui Tooltip component does not support the disabled prop which I didn't notice. Nice catch, I've fixed it.

@alexlyp alexlyp merged commit a89b903 into decred:master Feb 2, 2022
alexlyp pushed a commit that referenced this pull request Feb 14, 2022
@bgptr @alexlyp
[redesign] Settings view (#3631)
55c402d
@bgptr bgptr deleted the redesign-settings branch April 6, 2022 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexlyp alexlyp alexlyp approved these changes

@matheusd matheusd matheusd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.7.1
Development

Successfully merging this pull request may close these issues.

Settings views updates
3 participants
@bgptr @alexlyp @matheusd