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

Warning when formula is not applied should also trigger on Close button, not just Esc #7337

Closed
Andreya-Autumn opened this issue Nov 26, 2023 · 7 comments · Fixed by #7460
Closed

Warning when formula is not applied should also trigger on Close button, not just Esc #7337

Andreya-Autumn opened this issue Nov 26, 2023 · 7 comments · Fixed by #7460
Labels
Feature Request New feature request UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Milestone
Surge XT 1.3.1

Comments

@Andreya-Autumn
Copy link
Collaborator

Looking at the code for the previous issue this caught my eye:

https://github.com/mkruselj/surge/blob/76966ebf18405326a914ee3d9b1da302b9260c36/src/surge-xt/gui/overlays/LuaEditors.cpp#L864

I had not seen that dialog ever, since I click the x to close. Would be friendly to pop that window in this case also.
@Andreya-Autumn Andreya-Autumn added the Feature Request New feature request label Nov 26, 2023
@Andreya-Autumn Andreya-Autumn added this to the Surge XT 1.x milestone Nov 26, 2023
@Andreya-Autumn
Copy link
Collaborator Author

Oh hahaha. I also realized now that it's not working correctly. 🙃 Pressing yes, the window stays open.

@Andreya-Autumn Andreya-Autumn changed the title Formula un-applied changes warning also on x button Formula un-applied changes warning is borked (and would be nice to have on x too) Nov 26, 2023
@baconpaul baconpaul modified the milestones: Surge XT 1.x, Surge XT 1.3 Nov 29, 2023
@baconpaul
Copy link
Collaborator

Oh we should fix this for 1.3. On it.

@baconpaul
Copy link
Collaborator

Oh no it's harder to fix. Rebumping

@baconpaul baconpaul modified the milestones: Surge XT 1.3, Surge XT 1.x Nov 29, 2023
baconpaul added a commit to baconpaul/surge that referenced this issue Nov 29, 2023
@baconpaul
Formula Modulator Esc Key Handling Corrected
b501e63 
If you press esc in formula modulator

1. Yes/No box actually does yes/no properly and
2. If you chose No then escape again right away you get the
   box

Would be nice to have this on X, which is why this just
addresses surge-synthesizer#7337. The X treatment is trickier.
@baconpaul baconpaul mentioned this issue Nov 29, 2023
Merged
@baconpaul baconpaul changed the title Formula un-applied changes warning is borked (and would be nice to have on x too) Formula un-applied changes warning triggers on X not just esc Nov 29, 2023
@baconpaul
Copy link
Collaborator

Above PR fixes the esc key shenanigans at least which were a result of the alert box ports in 1.3

baconpaul added a commit that referenced this issue Nov 29, 2023
@baconpaul
Formula Modulator Esc Key Handling Corrected (#7345)
a768d2e 
If you press esc in formula modulator

1. Yes/No box actually does yes/no properly and
2. If you chose No then escape again right away you get the
   box

Would be nice to have this on X, which is why this just
addresses #7337. The X treatment is trickier.
@Andreya-Autumn
Copy link
Collaborator Author

Oh haha sorry, I misread you and un-re-bumped it. 😆 Let me re-un-re-bump.

@mkruselj mkruselj changed the title Formula un-applied changes warning triggers on X not just esc Warning when formula is not applied should also triggers on Close button, not just Esc Nov 30, 2023
@mkruselj mkruselj changed the title Warning when formula is not applied should also triggers on Close button, not just Esc Warning when formula is not applied should also trigger on Close button, not just Esc Nov 30, 2023
@mkruselj mkruselj added the UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc. label Dec 1, 2023
@mkruselj mkruselj modified the milestones: Surge XT 1.x, Surge XT 1.3.1 Dec 8, 2023
@baconpaul
Copy link
Collaborator

Figured out how to structure this. On it now.

@mkruselj
Copy link
Collaborator

Wahey!

baconpaul added a commit to baconpaul/surge that referenced this issue Jan 21, 2024
@baconpaul
Add an Overlay PreClose Chickenbox; Update Formula to use
5fa2c46 
1. Overlay component get std::optional getPreCloseChickenBoxMessage
   which if it returns a value pops a chicken box in the surge
   gui when you press 'x' on the overlay
2. Implement it for FormulaEditor so that 'x' and 'esc' do the
   same thing

Closes surge-synthesizer#7337
@baconpaul baconpaul mentioned this issue Jan 21, 2024
Merged
baconpaul added a commit that referenced this issue Jan 21, 2024
@baconpaul
Add an Overlay PreClose Chickenbox; Update Formula to use (#7460)
3f70b6c 
1. Overlay component get std::optional getPreCloseChickenBoxMessage
   which if it returns a value pops a chicken box in the surge
   gui when you press 'x' on the overlay
2. Implement it for FormulaEditor so that 'x' and 'esc' do the
   same thing

Closes #7337
This was referenced Mar 5, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Feature Request New feature request UX Issues related to user experience (UX) - mouse, touch, keyboard, MIDI inputs, etc.
Projects
None yet
Milestone
Surge XT 1.3.1
Development

Successfully merging a pull request may close this issue.

Add an Overlay PreClose Chickenbox; Update Formula to use baconpaul/surge
3 participants
@mkruselj @baconpaul @Andreya-Autumn