-
Notifications
You must be signed in to change notification settings - Fork 295
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
[DDW-1186] Stake pool descriptions is not fully shown if it is long enough #1832
[DDW-1186] Stake pool descriptions is not fully shown if it is long enough #1832
Conversation
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.
Looks good! 👍
I assume that it would not fit the minumum screen size 🤔 @nikolaglumac shouldn't we show only first few lines here? |
I can add ellipsis and limit the height of the description text. |
Will recheck @IuliaDolishniak , thats a good catch and definitely it should be fixed |
I assume that it would not fit the minumum screen size 🤔 @nikolaglumac shouldn't we show only first few lines here? https://zpl.io/a8p7ePJ - here is my proposal |
@a-rukin |
links requite to click, this feature is ok to be hidden, as we can afford this it's tooltip over popup dialog, so quite ok because first requires to click, second - to hover. As an option instead of "..." we can make gradient fadeout |
Yeah i understand what you are trying to say. But our component currently we use "pop" as you say, as a tooltip component and for us that's already a popup. That's why @daniloprates said it's strange to see tooltip on top of tooltip. |
Popup over popup is still ok. |
Other way around is to create stakepool page. Not sure you are ready to implement it now :D |
…ns-is-not-fully-shown-if-it-is-long-enough
@DeeJayElly @a-rukin @darko-mijic let's have a call regarding this one today! |
@nikolaglumac I'd like to participate as well if it didn't happen yet. |
Let me know what timeframe? We can talk now or you want to go through it on our weekly call? |
My suggestion: go for @a-rukin's proposal and think of a better overall solution. I have an idea that could integrate this issue with the Stake Pools comparison screen. |
…ns-is-not-fully-shown-if-it-is-long-enough # Conflicts: # CHANGELOG.md
@nikolaglumac no, I am talking regarding this one (cutoff tooltip), I didnt know the reslution on it. |
@IuliaDolishniak yes I am talking about the same one. Is that the only issue we have? |
@nikolaglumac from my side - yes. |
Ok. We will discuss what to do with that one tomorrow. @DmitriiGaico are there any other issues you are aware of? |
In general the PR looks good. But no approval yet, please don't ask how i found this. Same Pool for the PR's Build. Looks like something is wrong. |
…ns-is-not-fully-shown-if-it-is-long-enough
@DeeJayElly please update this line of the code: https://github.com/input-output-hk/daedalus/blob/v2-integration/source/main/config.js#L75 and increase the Please also remember to update these min-heights in storybook: https://github.com/input-output-hk/daedalus/blob/v2-integration/storybook/stories/_support/config.js#L44-L48 🙏 @IuliaDolishniak this will resolve the last remaining issue you have reported ☝️ |
@DeeJayElly please don't forget to check this one too: #1832 (comment) |
…ns-is-not-fully-shown-if-it-is-long-enough
… enough - fixes min window height
…ions-is-not-fully-shown-if-it-is-long-enough' into fix/ddw-1186-stake-pool-descriptions-is-not-fully-shown-if-it-is-long-enough
@DmitriiGaico can you somehow show me how did you managed to reproduce this issue? |
@DeeJayElly Sure, i just installed latest Rewards Build. For that moment it was 10535 and opened CLSD Pool. So no Data Injection or any other Manipulations. |
… enough - stake pool tooltip long words breaking
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.
@DeeJayElly I have left you a couple of comments...
storybook/stories/_support/config.js
Outdated
@@ -44,7 +44,7 @@ export const osNames: Array<any> = Object.keys(operatingSystems); | |||
export const osMinWindowHeights = { | |||
Windows: '541px', |
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.
@DeeJayElly you forgot to update the min-heights for Windows an Linux...
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.
Done
source/renderer/app/components/staking/stake-pools/StakePoolTooltip.scss
Outdated
Show resolved
Hide resolved
source/renderer/app/components/staking/stake-pools/StakePoolTooltip.scss
Show resolved
Hide resolved
…ions-is-not-fully-shown-if-it-is-long-enough' into fix/ddw-1186-stake-pool-descriptions-is-not-fully-shown-if-it-is-long-enough
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.
Great work @DeeJayElly 💅
This PR fixes stake pool descriptions in not fully shown if it's long enough.
Ticket: https://iohk.myjetbrains.com/youtrack/issue/DDW-1186
Screenshots
Testing Checklist
Review Checklist
Basics
feature
/bug
/chore
,release-x.x.x
)yarn test
)yarn dev
)yarn package
/ CI builds)yarn flow:test
)yarn lint
)yarn prettier:check
)yarn manage:translations
produces no changes)yarn storybook
)yarn.lock
file is updatedCode Quality
Testing
After Review
done
column on the YouTrack board