Skip to content

[GUI] Settings console, clear history button. #1499

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
Merged
furszy merged 1 commit into from
Apr 8, 2020
Merged

[GUI] Settings console, clear history button. #1499

furszy merged 1 commit into from
Apr 8, 2020

Conversation

@furszy
Copy link

@furszy furszy commented Apr 6, 2020

auto-descriptive PR.

Clear history button in the settings console.

Screen Shot 2020-04-06 at 2 17 01 AM

@furszy furszy self-assigned this Apr 6, 2020
Fuzzbawls
Fuzzbawls reviewed Apr 7, 2020
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

looking good. small nit regarding the new button's text string, but otherwise ACK

@Fuzzbawls Fuzzbawls added the GUI label Apr 7, 2020
@Fuzzbawls Fuzzbawls added this to the 4.1.0 milestone Apr 7, 2020
@random-zebra
Copy link

random-zebra commented Apr 8, 2020

Tested ACK from my side, once Fuzz nit is tackled.
Maybe in the future the - text can be replaced with a nice icon to instantly convey the purpose of the underlying action (a recycle bin, or something else...)

@random-zebra
Copy link

random-zebra commented Apr 8, 2020

Thinking more about it... do we really want to clear also the command history?
Wouldn't it be better to just clear ui->messagesWidget but skip history.clear()?
This way it would clean the messages, but still allow the user to use the up-arrow key to re-do previous commands.

@furszy
Copy link
Author

furszy commented Apr 8, 2020

Maybe in the future the - text can be replaced with a nice icon to instantly convey the purpose of the underlying action (a recycle bin, or something else...)

Yep. Had the same thought.

Wouldn't it be better to just clear ui->messagesWidget but skip history.clear()?

Agree. Changing it.

@furszy furszy force-pushed the 2020_clear_settings branch from 5608599 to 9dd5e7e Compare April 8, 2020 16:16
@furszy
Copy link
Author

furszy commented Apr 8, 2020

Updated.

random-zebra
random-zebra approved these changes Apr 8, 2020
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

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

ACK 9dd5e7e

@furszy furszy merged commit 5e6c053 into PIVX-Project:master Apr 8, 2020
JSKitty pushed a commit to ZENZO-Ecosystem/ZENZO-Core that referenced this pull request Apr 14, 2020
Backport PIVX-Project#1499 | Add clear history btn to console
2f13dca
@random-zebra random-zebra mentioned this pull request Apr 21, 2020
Closed
@Liquid369 Liquid369 mentioned this pull request Apr 30, 2020
Closed
@Fuzzbawls Fuzzbawls mentioned this pull request Jul 11, 2020
Merged
random-zebra added a commit that referenced this pull request Jul 14, 2020
@random-zebra
Merge #1744: [GUI] Recognize key event for clearing console
fda84ee 
ff9e3d8 [GUI] Recognize key event for clearing console (Fuzzbawls)

Pull request description:

  Alternative to #1585 that leaves the wording unchanged, but adds key-combo recognition to the settingsconsolewidget the clear the history as how the initial help text describes.

  Adjust the settingsconsolewidget event filter to explicitly recognize
  the key combo for clearing the console.

  fixes #1213 which was actually not adequately resolved in #1499

ACKs for top commit:
  random-zebra:
    tested ACK ff9e3d8
  furszy:
    utACK ff9e3d8

Tree-SHA512: a7ac42fbab600334ba7ecd94474140aefbf1063172a8e97e0ee0235f7d75c6c98a6d0cca5d685891cc81cfa653fa1e86c2c45be0d502698cfcc19a62dfe69428
@furszy furszy deleted the 2020_clear_settings branch November 29, 2022 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls left review comments

+1 more reviewer

@random-zebra random-zebra random-zebra approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@furszy furszy

Labels

GUI

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@furszy @random-zebra @Fuzzbawls