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.
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
GH-2066, GH-2068, GH-2069, GH-2082, GH-2067 Upgrade Plan panel subscription content #567
GH-2066, GH-2068, GH-2069, GH-2082, GH-2067 Upgrade Plan panel subscription content #567
Changes from 1 commit
901c4faf944c6ce035e69659749b2a83d42de02e6c53378f0a10bd6c7e552d2eb438329eb39bd2d2aa909383ab6f7b36a9f545e7b2fcc862ef915474fc3f532a9ffcbf120d4d7fe5a2433c506a0586ff5a77797c18167640847e49d25d313c9ad1de6398be1eb337bc3d51fd023e5eb749eb74947f1cd5ead0930cf7369301e96c8dd113ce4bd057b9e2774cc8c4606b4f891ee992df0d44147f5b223115406aaf03bdff3bdf4f300ddb508b74ada6783c7aa4e8ae40e71ce5901946f86ace9f20ddf4ce8a8859b81e4961924b67b52860da3eed5d7a559998c77a0ce34b03c3d0463b5299c6b0ffdb8e27751a342f94ccec7def0e9bf2e4dc5df6dcc24f016be27fa314ed61b117eb725611ba204cc6869844933880339dacf25464389210f72ebc30e4bb5dbf79481d8a48625630a843e41607004c4f1a4ad2af2dc1374bb728636602cd77ce79bc1861235f9d07e5004490a53fe165a1fd9929f5145a027289ab6aad0286954d9664e603b40a4d8ab52cee23898d0e7cefa42766aead069bc5d6997e6aca8def89b7507cc141bb276b95d2bd46b3364044f89d2a3b74af74097cf434038b4fb1ad18d029fec26b8cd6b547ed388f1fbf6761c561a2885b556dcbe6e1422fbd651e46bdebfc2888eaef4625fb6a529da16ed961e6553a4f7d464077d2a7da24b06ee180fb9f30eb7793File filter
Jump to
Add premiumAccess in hub. Already Protected buttons redirect to home …
wlycdgrJun 22, 2020
Member
There's a lot of duplication in the
return ( ....)code, which makes the code harder to read and modify than it needs to be and increases bug surface area. As the last push on this PR, let's DRY it out. As one example, instead of inlining all the feature matrix rows, we can define (at file scope) a render helper with the following signature:This makes it possible to condense the ~150 lines of inline
<tr>elements to:This is one of the main DRYing opportunities, but there are other big ones. At the least, we should do the same for the same batch of
<tr>elements for the mobile view. See if you can identify two or three other spots though! We don't need to get this code perfectly DRY, but we should wring it out at least SOME so it doesn't get that mildew smell.benstrumeyerJun 23, 2020
Author
Contributor
Yes, we can definitely do this! I wound up DRYing the matrix rows, cards, CTA buttons, and monthly/yearly toggle and it looks much cleaner now. I declared a few of those in component scope instead of file scope so they would have access to props--let me know if there's a better way to do this. As side note, I like how you're intentionally not passing params to those render helper functions so they evaluate to falsy, I used the same trick in a few of the other functions
wlycdgrJun 23, 2020
Member
Great! This is much cleaner, thank you.
Re: the ones you declared in component scope, with how many props / args we would need to pass to them, I think it's fine to leave them in there. If we get any complaints about performance during QA we know what to try, but I would be very surprised if we did.