-
Notifications
You must be signed in to change notification settings - Fork 206
fix: prevent list item jump on hover in virtual scrolling (v2) #4064
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
base: main
Are you sure you want to change the base?
Changes from all commits
db5e32f
762a8b2
b64be7d
0c3ba02
4bef83e
61eb48a
ab414f0
aded760
c0f72aa
c4b87ee
d664c47
095595a
17f0a2c
1f40836
42f1801
08f5f36
81919bb
6284fa8
9661ae0
68bbf77
c2ba79f
8e1497b
3b628c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,123 @@ | ||
| // Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import React, { useContext, useState } from 'react'; | ||
|
|
||
| import { Multiselect, MultiselectProps, SegmentedControl, SpaceBetween } from '~components'; | ||
| import Autosuggest, { AutosuggestProps } from '~components/autosuggest'; | ||
| import Select, { SelectProps } from '~components/select'; | ||
|
|
||
| import AppContext, { AppContextType } from '../app/app-context'; | ||
| import { SimplePage } from '../app/templates'; | ||
|
|
||
| type DemoContext = React.Context<AppContextType<{ type?: string }>>; | ||
|
|
||
| const options: SelectProps.Options = Array.from({ length: 1000 }, (_, i) => ({ | ||
| value: `${i}`, | ||
| label: `Option ${i + 1}`, | ||
| })); | ||
|
|
||
| const autosuggestOptions: AutosuggestProps.Options = Array.from({ length: 1000 }, (_, i) => ({ | ||
| value: `Option ${i + 1}`, | ||
| description: `Description for option ${i + 1}`, | ||
| })); | ||
|
|
||
| export default function () { | ||
| const { urlParams } = useContext(AppContext as DemoContext); | ||
| const [selectedType, setSelectedType] = useState(urlParams.type || 'select'); | ||
|
|
||
| const [selected, setSelected] = useState<SelectProps['selectedOption']>(null); | ||
| const [selectedMulti, setSelectedMulti] = useState<MultiselectProps.Options>(options.slice(0, 2)); | ||
| const [selectedMultiWithSelectAll, setSelectedMultiWithSelectAll] = useState<MultiselectProps.Options>([]); | ||
| const [autosuggestValue, setAutosuggestValue] = useState(''); | ||
|
|
||
| return ( | ||
| <SimplePage title="Virtual Scroll" i18n={{}} screenshotArea={{}}> | ||
| <div | ||
| style={{ | ||
| height: 500, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this height is no longer enough as there are more components now. I believe that for this to actually work as a screenshot testing page, these components probably need to be rendered one at a time, with some configuration (e.g. a segmented control above them)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The segmented control looks nice, but the selection state must use query parameters (see how it is done in other pages). This allows to avoid unnecessary steps in integration tests (instead one can open the page as e.g.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah I see, I would've clicked on the buttons. But that is way more convenient 👍 |
||
| padding: 10, | ||
| // Prevents dropdown from expanding outside of the screenshot area | ||
| overflow: 'auto', | ||
| }} | ||
| > | ||
| <SpaceBetween size="m"> | ||
| <SegmentedControl | ||
| selectedId={selectedType} | ||
| onChange={({ detail }) => setSelectedType(detail.selectedId)} | ||
| options={[ | ||
| { id: 'select', text: 'Select' }, | ||
| { id: 'multiselect', text: 'Multiselect' }, | ||
| { id: 'multiselect-select-all', text: 'Multiselect with Select All' }, | ||
| { id: 'autosuggest', text: 'Autosuggest' }, | ||
| ]} | ||
| /> | ||
|
|
||
| {selectedType === 'select' && ( | ||
| <Select | ||
| placeholder="Select with virtual scroll" | ||
| selectedOption={selected} | ||
| options={options} | ||
| filteringType="auto" | ||
| finishedText="End of all results" | ||
| onChange={event => setSelected(event.detail.selectedOption)} | ||
| virtualScroll={true} | ||
| expandToViewport={false} | ||
| ariaLabel="select demo" | ||
| data-testid="select-demo" | ||
| /> | ||
| )} | ||
|
|
||
| {selectedType === 'multiselect' && ( | ||
| <Multiselect | ||
| placeholder="Multiselect with virtual scroll" | ||
| selectedOptions={selectedMulti} | ||
| options={options} | ||
| filteringType="manual" | ||
| finishedText="End of all results" | ||
| errorText="verylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspacesverylongtextwithoutspaces" | ||
| recoveryText="Retry" | ||
| onLoadItems={() => {}} | ||
| onChange={event => setSelectedMulti(event.detail.selectedOptions)} | ||
| tokenLimit={2} | ||
| virtualScroll={true} | ||
| expandToViewport={false} | ||
| ariaLabel="multiselect demo" | ||
| data-testid="multiselect-demo" | ||
| /> | ||
| )} | ||
|
|
||
| {selectedType === 'multiselect-select-all' && ( | ||
| <Multiselect | ||
| placeholder="Multiselect with virtual scroll and select all" | ||
| selectedOptions={selectedMultiWithSelectAll} | ||
| options={options} | ||
| filteringType="auto" | ||
| finishedText="End of all results" | ||
| onChange={event => setSelectedMultiWithSelectAll(event.detail.selectedOptions)} | ||
| enableSelectAll={true} | ||
| virtualScroll={true} | ||
| expandToViewport={false} | ||
| ariaLabel="multiselect with select all demo" | ||
| data-testid="multiselect-select-all-demo" | ||
| /> | ||
| )} | ||
|
|
||
| {selectedType === 'autosuggest' && ( | ||
| <Autosuggest | ||
| value={autosuggestValue} | ||
| options={autosuggestOptions} | ||
| onChange={event => setAutosuggestValue(event.detail.value)} | ||
| enteredTextLabel={value => `Use: "${value}"`} | ||
| placeholder="Autosuggest with virtual scroll" | ||
| ariaLabel="autosuggest demo" | ||
| virtualScroll={true} | ||
| expandToViewport={false} | ||
| data-testid="autosuggest-demo" | ||
| /> | ||
| )} | ||
| </SpaceBetween> | ||
| </div> | ||
| </SimplePage> | ||
| ); | ||
| } | ||
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.
Why is this page nested under
selectable-item?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.
We're testing multiple selectables (autosuggest, select, multiselect). They all use selectable item, so it seemed like the best place to put it. What do you think?