Skip to content
This repository has been archived by the owner on Apr 26, 2022. It is now read-only.

Timeframe select #30

Closed
wants to merge 9 commits into from

Conversation

Ilnicki010
Copy link
Collaborator

No description provided.

@Ilnicki010 Ilnicki010 requested a review from rlueder June 21, 2020 15:02
@Ilnicki010 Ilnicki010 linked an issue Jun 21, 2020 that may be closed by this pull request
@Ilnicki010 Ilnicki010 changed the base branch from main to development June 21, 2020 15:02
@rlueder
Copy link
Owner

rlueder commented Jun 24, 2020

@Ilnicki010 thanks for taking this task. I made a couple small changes (adding hot reload so it's easier to tweak the component without a page refresh and enforced some stylelint rules).

I think there a two main things to try and solve here:

  1. I'd like the component to flow inline with the text so it reads as a sentence, only the last word happens to be a Select element, this is especially tricky when switching between device sizes (see screenshot of smaller devices attached);

  2. I'd like to be able to style it so that it "blends in" with the sentence as much as possible, I think the only way to achieve that is by using a completely custom component, it seems like React Select is the one option out there with most usage.

image

Let me know if you'd like to give this one another go or I can take it otherwise. Thanks!

@rlueder rlueder marked this pull request as draft June 24, 2020 13:19
@rlueder
Copy link
Owner

rlueder commented Jun 25, 2020

image

@rlueder
Copy link
Owner

rlueder commented Jun 25, 2020

And that seems to work, see this fiddle: https://jsfiddle.net/rlueder/982oegbz/23/ :)

@rlueder
Copy link
Owner

rlueder commented Jun 26, 2020

The flow of the Select component works for most screen sizes, as the window is resized it might still fall off screen but I think that's an edge case:

image

The position of the menu is now correct, it took quite some overwrites with !important rules to get it just right...

image

There is one main issue left before closing/merging this PR:

  • as the paragraph becomes longer/shorter depending on the value of Following__total the Select component might be overlapped by text (see screenshot below), I think the fix is to recalculate position of Select with each option selected on the menu, or with each update of Following__total I think either would work

image

@rlueder
Copy link
Owner

rlueder commented Jun 26, 2020

I actually just found another issue, the top property of Select__menu should take into account the height of Following__intro so the following rule hast to change:

.Select__menu {
  top: calc(
    #{$select_height} * 2 + 3px
  ); // to compensate for Select__control border-bottom
}

@rlueder
Copy link
Owner

rlueder commented Jun 26, 2020

hmm now getting a lot of these when resizing the window:

image

@rlueder
Copy link
Owner

rlueder commented Jun 30, 2020

I made progress, a few things remaining I want to finish today still:

  1. if the Select component is not completely in view it should not attach to position of :after element, I'll use the intersection observer API to determine this;

  2. If an option in the menu returns "0" accounts, time frame options after this option should not be displayed (e.g. if "3 months" returns 0 then, 6 months and one year should not be an option in the list, as they would necessarily also be 0), the total number would have to be calculated beforehand and not on selection as it happens now.

https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
https://blog.arnellebalane.com/the-intersection-observer-api-d441be0b088d

@rlueder rlueder deleted the branch rlueder:development August 12, 2021 14:07
@rlueder rlueder closed this Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

timeframe selection should be a Select component
2 participants