-
Notifications
You must be signed in to change notification settings - Fork 479
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
Music: use sounds panel revision #58655
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 🚀
// Capture whether the initially-selected sound is the default sound, in which | ||
// case we will do a smooth scroll into view, rather than scroll directly | ||
// to the current sound. | ||
const isDefaultSoundSelected = useRef( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To check my understanding: we're using useRef
so that the below useEffect
only runs on initial render, and not whenever currentValue
changes, right? If so, mind updating the comment to capture that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's right. Comment updated.
With this change, the revised sounds panel introduced in #58275 becomes the default.
Two UI issues have been fixed:
The
ScrollIntoView
component has been renamedEaseIntoView
to alleviate confusion with the browser's existingscrollIntoView
function.A DCDO flag of
"music-lab-use-sounds-panel-1"
can be set totrue
or thesounds-panel-2=false
URL parameter can continue to be used to switch back to the legacy sounds panel.