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

Dropdown: elements min width #755

Closed
wants to merge 4 commits into from
Closed

Conversation

rperez89
Copy link
Contributor

@rperez89 rperez89 commented Apr 11, 2020

Not sure if this is the desired behavior but opening to start the discussion

Issue : aragon/aragon-apps#1047

@auto-assign auto-assign bot requested a review from bpierre April 11, 2020 03:49
Copy link
Contributor

@sohkai sohkai left a comment

Choose a reason for hiding this comment

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

I still contend the issue is likely to with the automatic size detection, with the specific adjustment that was removed in https://github.com/aragon/aragon-ui/pull/608/files#diff-55232f9d6a5dbb62a16b3f3e556275cbL126-L127 being relevant.

But I think @bpierre will have some thoughts about this as well

@@ -186,7 +178,7 @@ const DropDown = React.memo(function DropDown({
padding-left: ${2 * GU}px;
padding-right: ${1.5 * GU}px;
width: ${width || (wide ? '100%' : 'auto')};
min-width: ${wide ? 'auto' : `${placeholderMinWidth}px`};
min-width: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need this to be set directly, because using auto won't automatically adjust for the width of all items.

The original behaviour was intended for the dropdown's width to fit all listed items, even if they were not selected.

@sohkai sohkai marked this pull request as draft April 12, 2020 16:13
@@ -9,8 +9,6 @@ import { useViewport } from '../../providers/Viewport/Viewport'
import ButtonBase from '../ButtonBase/ButtonBase'
import Popover from '../Popover/Popover'

const MIN_WIDTH = 128
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we try and use GU here?

setMeasureWidth(false)
}, [])
useEffect(() => {
setMeasureWidth(true)
}, [vw, items])

console.log('measured width ', measureWidth)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remember to remove this when this is ready to review. 😃

@stale stale bot added the abandoned label Jun 3, 2020
@aragon aragon deleted a comment from stale bot Jun 3, 2020
@stale stale bot removed the abandoned label Jun 3, 2020
@stale stale bot added the abandoned label Jul 4, 2020
@Evalir Evalir removed the abandoned label Jul 4, 2020
@aragon aragon deleted a comment from stale bot Jul 4, 2020
@stale stale bot added the abandoned label Aug 3, 2020
@aragon aragon deleted a comment from stale bot Aug 3, 2020
@stale stale bot removed the abandoned label Aug 3, 2020
@stale stale bot added the abandoned label Sep 2, 2020
@aragon aragon deleted a comment from stale bot Sep 2, 2020
@stale stale bot removed the abandoned label Sep 2, 2020
@stale stale bot added the abandoned label Oct 3, 2020
@aragon aragon deleted a comment from stale bot Oct 5, 2020
@stale stale bot removed the abandoned label Oct 5, 2020
@stale stale bot added the abandoned label Nov 5, 2020
@aragon aragon deleted a comment from stale bot Nov 6, 2020
@stale stale bot removed the abandoned label Nov 6, 2020
@stale stale bot added the abandoned label Dec 7, 2020
@aragon aragon deleted a comment from stale bot Dec 7, 2020
@stale stale bot removed the abandoned label Dec 7, 2020
@stale
Copy link

stale bot commented Jan 10, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for contributing to Aragon! 🦅

@stale stale bot added the abandoned label Jan 10, 2021
@stale stale bot closed this Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants