Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Landing page's
SeasonCountdownSection
component #368Landing page's
SeasonCountdownSection
component #368Changes from 19 commits
ea11fbb
322644a
ff59bb6
61a9503
f60dfb8
076d2ce
e962925
a4ba563
e958c8e
102a2d8
2df35a3
d87bc51
7f39cce
b876889
32db87b
bff469a
1087104
789bcac
eb3cb02
a26e6ea
5796c8b
99ba8fb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Are we going to address it in a separate PR? Or maybe it's already addressed?
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.
It's a barely noticeable issue. Most of ppl won't even notice.
I gave it a shot and it looks like we can't do much with it. It's browser specific and depends on how the browser (Chromium in Ledger Live's case) processes SVG transformations.
Let's leave it for possible future consideration.
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.
If we want to spend more time on the investigation in the future let's create an issue and add it to the list at #154 EPIC.
If there is nothing more we can do let's live with it and remove the TODO.
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
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 would consider moving the
SeasonCountdownSection
dir to thepages/LandingPages/components
because it is rather not a shared component but specific for landing page.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.
Ref commit: a26e6ea
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 agree that it should be in the
pages/LandingPages
directory but I am not convinced about nesting it in the components directory. More info here.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 would suggest to change the name of this directory. More information here.
SeasonCountdownSection
as a directory has no exports and likely will never export a component with this name. If we can let's use lowercasing directory names like this that are purely categorical.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 don't quite understand what's going on here. 😅 I don't quite understand why we set
maxW
.Why can't we do it like this? 🤔