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

Dance AI: Add start over button #54899

Merged
merged 3 commits into from
Nov 14, 2023

Conversation

sanchitmalhotra126
Copy link
Contributor

@sanchitmalhotra126 sanchitmalhotra126 commented Nov 14, 2023

Adds a "Start Over" icon button to the top left corner. As per the mocks, this is present on the results page, and disabled on the explanation page. Does not appear on any other pages in the modal.

Note that we may end up changing the icon (I personally found it a bit confusing at first, but also understand that it needs to contract with start over) but that change should just be a matter of changing the icon class name(s).

Also made a few related quality of life changes as I noticed them

  • Extracted conditional button logic into a separate ModalButton component - this might be a bit verbose, but it gives us a way to more cleanly conditionally show/hide/disable buttons based on the current modal mode. Happy to discuss though if folks feel that it would be better to keep the logic inside DanceAiModal.
  • Moved icons and text that were previously component children into Button props for consistency
  • Added aria-labels for icon buttons

Results pane:
Screenshot 2023-11-14 at 11 54 29 AM

Voiceover readout for Start Over button:
Screenshot 2023-11-14 at 11 55 58 AM

Voiceover readout for Explanation button:
Screenshot 2023-11-14 at 11 56 18 AM

RTL Results pane:
Screenshot 2023-11-14 at 3 49 42 PM

Links

https://trello.com/c/PEcuk8tz

Testing story

Tested locally.

onClick={handleExplanationClick}
color={Button.ButtonColor.neutralDark}
className={moduleStyles.button}
aria-label={i18n.danceAiModalExplanationButton()}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@breville just confirming this was the intended label for this button

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I believe so.

@breville
Copy link
Member

Let's just hide the "Start over" button on the explanation pane, since we also hide all the other buttons.

@breville
Copy link
Member

Should we have the icon-only buttons show their alt-text on hover?

@sanchitmalhotra126
Copy link
Contributor Author

Should we have the icon-only buttons show their alt-text on hover?

Oh good call, will update.

Copy link
Member

@breville breville left a comment

Choose a reason for hiding this comment

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

Ran locally and looks good. Love the new button component!

Copy link
Collaborator

@levadadenys levadadenys left a comment

Choose a reason for hiding this comment

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

LGTM!
Nice idea to add an optional title so that icon only button can have a tooltip, thanks!

@sanchitmalhotra126 sanchitmalhotra126 merged commit 243cf1f into staging Nov 14, 2023
2 checks passed
@sanchitmalhotra126 sanchitmalhotra126 deleted the sanchit/dance-ai-start-over-button branch November 14, 2023 23:35
molly-moen added a commit that referenced this pull request Nov 15, 2023
* Tutorial explorer: preview mode on hourofcode.com

* Updated test to account for flakiness.

* Added comment

* Re-enabled test

* hoc2023: AI string updates

* Use use_preview variable

* Update schema cache dump after schema changes.

* Require script_id for SingleSectionExperiment and TeacherBasedExperiment (#54843)

* require script_id on SingleSectionExperiment and TeacherBasedExperiment

* fix and improve unit tests

* fix unit tests

* revert stray schema changes

* Update string and font size

* Split lines for explanation text.

* update injection options (#54818)

* Handle pending and error states for rubric ai evaluation (#54860)

* updated rubric for more useful error messages

* updated rubric settings tests for new errors

* undo debug changes to rubrics_controller

* bug fixes

* added messages for pending and running states

* Dance AI: Add start over button (#54899)

* Dance AI: Add start over button

* don't show on explanation page, add tooltips

* Updates to instructions in HoC progression (#54881)

Updates to instructions in HoC progression

* Allow setting workshop organizer permission (#54896)

* Allow setting workshop organizer permission

* Update Assignable Permissions

* added categories and standards csv files (#54933)

* levelbuilder content changes (-robo-commit)

* staging content changes (-robo-commit)

* * removed empty files on dashboard/public/fonts/gotham (#54906)

* P20-522: Fix Express22 level (#54789)

* P20-522: Translate `gamelab_ifVarEquals` block type NUM vsriables

* P20-522: Translate `procedures_defnoreturn` block id attr

* Revert "P20-522: Translate `procedures_defnoreturn` block id attr"

This reverts commit e0f4280.

* [DESign2-51] Add Chips component (#54064)

* Created Chips component
* created stories for Chips
* created tests for Chips
* created documentation for Chips
* removed MultiSelectGroup
* used Chips instead of MultiSelectGroup in the code

* Teachers find course button goes to catalog (#54918)

* Teachers find course button goes to catalog

* update test

* staging content changes (-)

* HoC 2023 - Fix missing string on Educators How-to guide (#54932)

* Add skinny banner to code.org/student/middle-high for AFE scholarships (#54907)

* refactor middle-high.haml and remove stylesheet

* add afe_scholarship_skinny_banner.haml view

* update grade labels and add empty alt text to images

* Dance AI: handle RTL in header, footer, and effect/code toggle (#54922)

* * add focus state for toggle code/effect buttons

* * dance ai - explanation area RTL support fixes

* * dance party - song selector RTL support fix

* * updated explanation button rtl languages positioning
* added comment (review fix)

* Center toggle in LTR/RTL, force LTR header even in RTL languages

* RTL header

* Handle RTL in footer buttons

* Fix text/icon spacing in RTL

* Undo spacing change

* Use div instead of span

---------

Co-authored-by: denyslevada <levada.denys@gmail.com>

* standardize counting of events so all are right (#54927)

* Update Neural Networks lesson text on code.org/ai/how-ai-works and hourofcode.com/ai (#54915)

* Update code.org/hourofcode see all Hour of code tutorials section (#54931)

* add see all activities section

* update images and add responsive styles

---------

Co-authored-by: breville <brendan@code.org>
Co-authored-by: Jessica Kulwik <jessica@code.org>
Co-authored-by: Dave Bailey <davidsbailey@users.noreply.github.com>
Co-authored-by: Molly Moen <molly@code.org>
Co-authored-by: Mark Barnes <mark.barnes@code.org>
Co-authored-by: Sanchit Malhotra <85528507+sanchitmalhotra126@users.noreply.github.com>
Co-authored-by: fisher-alice <107423305+fisher-alice@users.noreply.github.com>
Co-authored-by: Brendan Reville <breville@users.noreply.github.com>
Co-authored-by: Dani LaMarca <dani@code.org>
Co-authored-by: levadadenys <levada.denys@gmail.com>
Co-authored-by: Artem Vavilov <11708250+artem-vavilov@users.noreply.github.com>
Co-authored-by: Kelby Hawn <9256643+kelbyhawn@users.noreply.github.com>
Co-authored-by: bencodeorg <ben@code.org>
Co-authored-by: Turner Riley <56283563+TurnerRiley@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants