Skip to content

Fix error msg #719

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
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix error msg #719

wants to merge 1 commit into from

Conversation

@lievenhey
Copy link
Contributor

@lievenhey lievenhey commented Apr 28, 2025

No description provided.

@lievenhey
fix: error message not shown on record page
c28b5b8 
This patch stops hotspot from clearing the error message. Resetting the
error message in a happy path is a terrible idea since it might remove
errors that occurred earlier.

Fixes: #711
@lievenhey lievenhey requested a review from milianw April 28, 2025 12:36
@lievenhey lievenhey linked an issue Apr 28, 2025 that may be closed by this pull request
When perf isn't in $PATH, the "Start Recording" button in "File" > "Record Data" is disabled. However, the reason isn't communicated to the user. #711
Open
@milianw
Copy link
Member

milianw commented Apr 28, 2025

how do you ensure that the error message is cleared once a valid value is entered?

@lievenhey
Copy link
Contributor Author

lievenhey commented Apr 28, 2025

Good question, with the current flow it is not possible. We would need a isValid check or something like that. Then we could also push all messages on a stack and show them all at once.

@milianw
Copy link
Member

milianw commented May 7, 2025

Then please revisit this patch, imo it doesn't fix the bug in the minimal approach and leads to regressions for validation of other fields

milianw
milianw requested changes May 7, 2025
View reviewed changes
Copy link
Member

@milianw milianw left a comment •
edited
Loading

Choose a reason for hiding this comment

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

see previous comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@milianw milianw milianw requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

When perf isn't in $PATH, the "Start Recording" button in "File" > "Record Data" is disabled. However, the reason isn't communicated to the user.

3 participants

@lievenhey @milianw