-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[MBL-1815] PLOT plan selector - selected state #2213
Changes from 8 commits
7fda0e7
5169766
ed401ec
eff7fa4
b5aab6c
43bd0a5
5db01cb
2bcd664
8893498
8589d20
d38cfa3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like there may be some extra spacing after "Charge 1". I know we were having issues without iPad snapshots for this previously. If it's the same issue here, that's fine for now. I'd like to fix that, though, if possible. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, we need to address how we handle snapshot tests. I’ll create a ticket for this purpose. You can follow this thread for more details. |
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.
The spacing issue shown here is due to the test locking the layout to a specific screen size, which might not fully represent the component's dynamic behavior. In actual implementation, this component adapts its size based on the content and available screen dimensions, so the extra space seen in the test is expected.
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.
Can you link the ticket you created here for reference? We should update how we use snapshot tests if we make a habit of testing individual views instead of the entire screen. (I'm a big fan of testing individual views, for the record; we just don't do that currently)
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.
I haven’t created a ticket for this yet, but I can definitely do so to track this issue and the potential update to how we approach snapshot testing. Let me know if that works!