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

Show several popups using popperjs #1139

Merged
merged 10 commits into from
Jul 15, 2021
Merged

Conversation

davidyuk
Copy link
Member

@davidyuk davidyuk commented Jul 9, 2021

depends on #1138

Extracting into separate components:

  • UserPopup (Popperjs that used in Bootstrap makes it drop up if no space at the bottom, see screenshot)
  • FeedItemMenu (extracts some logic from TipRecord)
  • TokenSelect popup (makes to use the same component in 3 places, @onvisions is it a design requirement for them to be different?)
  • TipInputPopup (extracts some logic from TipInput that is not used in deeplink use case)
  • CookiesDialog

Screenshot 2021-07-09 at 21 24 51

@Liubov-crypto please ensure that mentioned components work correctly

@github-actions
Copy link

github-actions bot commented Jul 9, 2021

@davidyuk davidyuk changed the title refactor(Author): show UserPopup using popperjs Show several popups using popperjs Jul 10, 2021
@davidyuk davidyuk changed the base branch from develop to feature/tip-plain-link July 10, 2021 23:56
@davidyuk davidyuk changed the base branch from feature/tip-plain-link to develop July 10, 2021 23:58
@davidyuk davidyuk force-pushed the feature/refactor-dropdowns branch from de09e7d to d8f4a19 Compare July 11, 2021 13:33
@davidyuk davidyuk marked this pull request as ready for review July 13, 2021 01:01
@AtanasKrondev
Copy link
Contributor

There's some weird flicker on the cookies popup while you are scrolling.

flick

@davidyuk
Copy link
Member Author

@AtanasKrondev I've fixed it by disabling a popper plugin that is enabled by default

Copy link
Contributor

@AtanasKrondev AtanasKrondev left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Collaborator

@Liubov-crypto Liubov-crypto left a comment

Choose a reason for hiding this comment

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

Tested, in general LGTM.

Found this on the testnet:
After the retip transaction, the long token name goes out of the feed boundary.
long token name
test t

The wallet seems to have a broken dropdown (scroll doesn't work), I see fewer tokens than the dropdown below the post:
broken drop-down menu in wallet

@davidyuk
Copy link
Member Author

The wallet seems to have a broken dropdown (scroll doesn't work), I see fewer tokens than the dropdown below the post

cc: @etharner

@davidyuk
Copy link
Member Author

I've fixed TokenSelector by adding space between token name and amount

@davidyuk davidyuk merged commit ec25196 into develop Jul 15, 2021
@davidyuk davidyuk deleted the feature/refactor-dropdowns branch July 15, 2021 00:49
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.

3 participants