Skip to content

[Poetry] Updates Select Poem dropdown to use component library simpleDropdown #70710

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

Merged
hannahbergam merged 1 commit into from
Feb 11, 2026
Merged

[Poetry] Updates Select Poem dropdown to use component library simpleDropdown #70710

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions apps/src/p5lab/poetry/PoemSelector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import SimpleDropdown from '@code-dot-org/component-library/dropdown/simpleDropdown';
import PropTypes from 'prop-types';
import React, {useState, useEffect} from 'react';
import {connect} from 'react-redux';
import Select from 'react-select';

import 'react-select/dist/react-select.css';
import project from '@cdo/apps/code-studio/initApp/project';
import fontConstants from '@cdo/apps/fontConstants';
import StylizedBaseDialog, {
Expand Down Expand Up @@ -161,7 +160,7 @@ function PoemSelector(props) {
};

const onChange = e => {
const poemKey = e.value;
const poemKey = e.target.value;
if (poemKey === msg.enterMyOwn()) {
setIsOpen(true);
return;
Expand Down Expand Up @@ -195,11 +194,11 @@ function PoemSelector(props) {
if (shouldAlphabetizePoems()) {
options.sort((a, b) => (a.title > b.title ? 1 : -1));
}
options = options.map(poem => ({value: poem.key, label: poem.title}));
options = options.map(poem => ({value: poem.key, text: poem.title}));
// Add option to create your own poem to the top of the dropdown.
options.unshift({value: msg.enterMyOwn(), label: msg.enterMyOwn()});
options.unshift({value: msg.enterMyOwn(), text: msg.enterMyOwn()});
// Add blank option that just says "Choose a Poem" to the top of the dropdown.
options.unshift({value: msg.chooseAPoem(), label: msg.chooseAPoem()});
options.unshift({value: msg.chooseAPoem(), text: msg.chooseAPoem()});
return options;
};

Expand All @@ -216,17 +215,15 @@ function PoemSelector(props) {
handleClose={handleClose}
initialPoem={initialEditorPoem()}
/>
<label>
<b>{msg.selectPoem()}</b>
</label>
<div style={styles.selector}>
<Select
value={props.selectedPoem.key}
clearable={false}
searchable={false}
<div style={styles.dropdownWrapper}>
<SimpleDropdown
Copy link
Contributor

@fisher-alice fisher-alice Feb 11, 2026

Choose a reason for hiding this comment

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

I'm noticing that SimpleDropdown doesn't seem to be scrollable. I wonder if this may be an issue for poetry levels with long poem lists on mobile devices.
I see that for Dance (legacy) levels, the SongSelector is scrollable on mobile (checked on my phone!)
I see that they both use native <select> so it seems like there's some styling that's determining this in SimpleDropdown? I wonder if this is intended for SimpleDropdown or not. @moshebaricdo do you have context here?

Copy link
Contributor

@moshebaricdo moshebaricdo Feb 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

From what I'm seeing both use native select as the base, but the legacy implementation is a react select component with bootstrap inspired custom styling for the button trigger and dropdown menu. SimpleDropdown in DSCO uses a fully native approach for the dropdown menu and only applies custom styles to the select element itself (the button trigger).

When we were deciding whether to use a custom dropdown menu styling or go fully native, we opted to go fully native as that is best practice for the most reliable/accessible approach and most flexible as users in any environment will see their OS/browser's native menu—even though it's not as pretty. This is actually better for mobile, as they would see the iOS or android menu when opening the dropdown vs a custom styling.

Copy link
Contributor

@fisher-alice fisher-alice Feb 11, 2026

Choose a reason for hiding this comment

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

Thanks Moshe! So my concern was that for a dropdown with many options, a user could still scroll through entire list on a smaller mobile device. Can you confirm that a user can scroll in SimpleDropdown?

Copy link
Contributor

@moshebaricdo moshebaricdo Feb 11, 2026

Choose a reason for hiding this comment

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

Yep! On an iPad, for example, they'd see iOS's native dropdown which has its own optimizations/styling in place for menus with lots of options vs just a few. That was the primary driver for taking this approach for simple dropdown. I made a super quick codepen experiment to show what this would look like on iOS.

Image

Copy link
Contributor

@fisher-alice fisher-alice Feb 11, 2026

Choose a reason for hiding this comment

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

The reason I asked is that locally when I checked out Hannah's branch, when I selected the iPhone view, I saw this dropdown which wasn't scrollable.

Image

However, I found a place where SimpleDropdown is used with a long list - StudentSelector is a student dropdown on teacher dashboard unit page! I logged in on my iPhone, and viewed the list from a section with >30 students and list is scrollable. 🎉

Copy link
Contributor

@fisher-alice fisher-alice Feb 11, 2026

Choose a reason for hiding this comment

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

Yep! On an iPad, for example, they'd see iOS's native dropdown which has its own optimizations/styling in place for menus with lots of options vs just a few.

Thanks! I posted my reply above before seeing your latest response. Appreciate the explanation and the check!

Copy link
Contributor

@moshebaricdo moshebaricdo Feb 11, 2026

Choose a reason for hiding this comment

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

Of c! My delay was also trying to find a good example I could screenshot on my phone 😄

Copy link
Contributor Author

@hannahbergam hannahbergam Feb 11, 2026

Choose a reason for hiding this comment

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

Thank you both for the learnings! I agree Alice that I usually test mobile by 'hacking' a mobile view in the browser (which in this case, doesn't quite work)

name="poem-selector"
labelText={msg.selectPoem()}
items={getPoemOptions()}
selectedValue={props.selectedPoem.key}
onChange={onChange}
options={getPoemOptions()}
disabled={props.isRunning}
dropdownTextThickness="thin"
/>
</div>
</div>
Expand All @@ -245,8 +242,7 @@ const styles = {
container: {
maxWidth: APP_WIDTH,
},
selector: {
width: '100%',
dropdownWrapper: {
marginBottom: 10,
},
label: {
Expand Down