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.

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

Pause icon in paragraph-control is not clickable but disabled #204

Open
ronja-ui opened this issue Jun 22, 2023 · 2 comments
Open

Pause icon in paragraph-control is not clickable but disabled #204

ronja-ui opened this issue Jun 22, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@ronja-ui
Copy link
Contributor

In paragraph-control when you click run paragraph button, it shows a pause button that looks clickable but actually isn't.

<i class="fas fa-pause" role="button" title="Cancel (Ctrl+{{ (isMac ? 'Option' : 'Alt') }}+C)" ng-click="cancelParagraph(paragraph)" ng-class="{'item-disable' : isNoteRunning}"> </i>

When .item-disable class name (which is not the best name) is attached to the icon, it makes the pause icon light blue. When there's no .item-disable class name, the pause icon is red.

Sometimes the pause icon is disabled, sometimes it's not. It's confusing for the user.

Investigate how the feature is implemented and figure out a better UX for it.

@ronja-ui ronja-ui added the enhancement New feature or request label Jun 22, 2023
@ronja-ui ronja-ui self-assigned this Jun 22, 2023
@BVVLD
Copy link

BVVLD commented Jul 2, 2024

the code related to this issue is highly spaghettified. Currently, there are three different methods in paragraph controller:
isNoteRunning, isRunning and isParagraphRunning.
not related to the issue: (isRunning is just an interface for isParagraphRunning utility, interconnected and dependent to paragraph-status logic placed elsewhere. )
isNoteRunning, however, is a scope variable, that is changed during several conditions:

  • it is false by default;
  • during init phase, it takes a result of logical statement "!!(note && note.hasOwnProperty('info') && note.info.hasOwnProperty('isRunning') && note.info.isRunning === true)" which means, that note object (provided by server) can have property "info" with subproperty "isRunning" and if it is set to true, then isNoteRunning will be true. Otherwise - false;
  • if anything broadcasts the event "noteRunningStatus" with supplied status, it will change the value to that status. Currently, the only way to get this event is if server sends a websocket call "NOTE_RUNNING_STATUS";
    These are only three conditions, that can change the isNoteRunning state. Therefore, all note running data is supplied by server and UI has no intervention to it.

What else does isNoteRunning do? Quite a lot, actually:

  • it sets editor to read-only mode if set to true, but only during "getEditor" call
  • if it is true, prevents cancelParagraph() call. Therefore, it is not BUTTON DISABLED, but button CAN WORK in both disabled and enabled state. It will only not allow it to work in disabled state via JS logic.
  • if it is true, it prevents paragraph config toggling. Meaning, if it is open, it will not close and vice versa.
  • if it is true, it prevents running paragraph from paragraph run button (same as cancel - via JS logic)
  • the same fashion as above, it prevents running "runAllToThis", "runAllFromThis", "runAllFromThisFromShortcut", "runAllToThisFromShortcut" (these are conditional paragraph runs, not sure if used anywhere), "moveUp", "moveDown" (paragraph displacement), "insertNew", "removeParagraph" and "copyPara" (and new paragraph creation/cloning/removing) functions.
  • after ACE editor throws "loaded" event, it will set the editor config to read-only, if it is true
  • the same will happen to ACE, when paragraph object is updated

Additionally, the "cancelParagraph(paragraph)" function, which is called by the button click, is only doing one thing: it sends the websocket call "CANCEL_PARAGRAPH" with paragraphID provided. It does not have any additional logic in it and has no feedback. Therefore, if call is sent, but ignored/returns error (and we know how current error handling is performing) - user will see nothing and will think that button is unclickable.

@BVVLD
Copy link

BVVLD commented Jul 15, 2024

Current decision:
Refactor as much of a code as possible to make it more elegant (following guidelines from the book). Possibly, leave only one combined object isRunning, which should be able to correctly respond on any of the cases.
The server implementation of the paragraph cancelling should also be refactored to provide more functionality.

User stories:
As a user, I should be able to stop the run almost immediately if I wish and not wait till that possibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants