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

feat: show dropdown on top if input is near viewport bottom #204

Merged
merged 3 commits into from
Jan 25, 2023
Merged

Conversation

brc-dd
Copy link
Member

@brc-dd brc-dd commented Jan 24, 2023

fixes #200

Demo (doesn't seem smooth though 👀):

Screen.Recording.2023-01-24.at.9.55.22.PM.mov

@netlify
Copy link

netlify bot commented Jan 24, 2023

Deploy Preview for sefirot-story ready!

Name Link
🔨 Latest commit cef476a
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-story/deploys/63d0a8d349544c00090ba2cd
😎 Deploy Preview https://deploy-preview-204--sefirot-story.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Jan 24, 2023

Deploy Preview for sefirot-docs ready!

Name Link
🔨 Latest commit cef476a
🔍 Latest deploy log https://app.netlify.com/sites/sefirot-docs/deploys/63d0a8d31ce62d0008497e6e
😎 Deploy Preview https://deploy-preview-204--sefirot-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Base: 68.03% // Head: 67.79% // Decreases project coverage by -0.24% ⚠️

Coverage data is based on head (cef476a) compared to base (d49b07b).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #204      +/-   ##
==========================================
- Coverage   68.03%   67.79%   -0.24%     
==========================================
  Files         105      105              
  Lines        8324     8353      +29     
  Branches      322      322              
==========================================
  Hits         5663     5663              
- Misses       2661     2690      +29     
Impacted Files Coverage Δ
lib/components/SInputDropdown.vue 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kiaking
Copy link
Member

kiaking commented Jan 25, 2023

Looking great! I've updated the code to only calculate the position on the open event and stay the same during the scroll. I think it's a bit more intuitive for the user... maybe. But it's good to know making it work on scroll is easy as pie with vueuse!

Also, I've added a limit to the upper space. So, if we don't have enough space at the top, display the dialog at bottom always. This might happen a lot in mobile device or when user have quite short window size.

Could you review my changes and see if didn't do anything stupid here 👀

The code looks like this now.

const { top, bottom } = useElementBounding(container)
const { height } = useWindowSize()

const pos = ref<'top' | 'bottom'>('bottom')

async function handleOpen() {
  if (!props.disabled) {
    pos.value = getPosition()
    open()
  }
}

function getPosition() {
  if (props.position) {
    return props.position
  }

  const dialogHeight = 400

  // If the space top of the input is not enough to show dialog, just show
  // the dialo at the bottom of the input.
  if (top.value < dialogHeight) {
    return 'bottom'
  }

  // Else show dialog depending on the space bottom of the input.
  return bottom.value + dialogHeight <= height.value ? 'bottom' : 'top'
}

@brc-dd
Copy link
Member Author

brc-dd commented Jan 25, 2023

Yeah seems fine 👍

I had to bump vueuse because that scroll thing got fixed last month 😅

@kiaking
Copy link
Member

kiaking commented Jan 25, 2023

Thanks!

@kiaking kiaking merged commit 1084ae5 into main Jan 25, 2023
@kiaking kiaking deleted the fix/200 branch January 25, 2023 04:29
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.

Auto position dropdown dialog for SInputDropdown
2 participants