-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$1000] Separator line removes margin from either side on hover #17535
Comments
Triggered auto assignment to @isabelastisser ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Separator line removes margin from either side on hover What is the root cause of that problem?We're using horizontal padding for What changes do you think we should make in order to solve the problem?This happens for both Language and Priority Mode Pages. optionHoveredStyle={styles.hoveredComponentBG} DemoScreen.Recording.2023-04-19.at.8.19.58.PM.mov |
Reviewing this now. |
I cannot reproduce this! |
@isabelastisser I'm still able to reproduce this on production (v1.3.3-2) Screen.Recording.2023-04-24.at.6.38.42.PM.mov |
@isabelastisser See the Left and Right side of Separator line between Most recent & #focus You will see when I hover First item it shows Margin for separator from both Right and Left and when I hover on Second item there is no margin from Right and Left. Screen.Recording.2023-04-27.at.2.26.29.PM.mov |
I see this too still :D |
Job added to Upwork: https://www.upwork.com/jobs/~01627ca117ea2d2ea0 |
Current assignee @isabelastisser is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat ( |
I don't think this is a bug, and we are actually going to just get rid of the separator line with some upcoming list refactors. So I say let's just close this out. |
Aah ok @shawnborton - do you know if we have any issue we can link here to show the plan? Maybe not necessary but could be helpful |
I believe this is it: #11795 |
ProposalPlease re-state the problem that we are trying to solve in this issue.Separator line removes margin from either side on hover What is the root cause of that problem?The root cause of the problem seems to be related to the inconsistent dimensions between the hover box and the specified borderTop styling. This inconsistency results in the outlined border extending beyond its intended size when the hover event occurs.
What changes do you think we should make in order to solve the problem?If you want to ensure that the outlined component's dimensions remain unaffected when hovering, one solution is to create a dedicated component specifically for the outlined border. This way, the dimensions of the outlined component will remain consistent, and you can apply hover effects to a separate component without impacting the outlined border.
Before FixedScreencast.from.23-06-23.11.53.17.PM.IST.webmAfter FixedScreencast.from.23-06-23.11.54.45.PM.IST.webm |
A simple fix is to remove a border-top (as a separator) style from "PressableWithFeedback" to the child "View" component at OptionRow.js:
This should fix, please take a look at the video. Serhiy.Kochkin.mov |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
Separator should maintain margin from left and right
Actual Result:
Separator line removes margin from either side on hover.
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Screen.Recording.2023-04-16.at.7.28.25.PM.mov
Recording.254.mp4
Expensify/Expensify Issue URL:
Issue reported by: @DinalJivani
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681653552296999
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: