Overall improvements #212
Merged
Overall improvements #212
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Not a fan of the Inlang localization step in load functions but yeah...
10x faster build time 8x less modules to transform
- Improve callback handling (in a very smart way tbh) - Fix click outside not closing it - Fix "undefined" in the DOM - Fix useless use of svelte:fragment
WarningImHack3r
commented
Feb 16, 2024
There was a problem hiding this comment.
Explanation of that very special component communication mechanism. It allows for a JS callback, within the markup, without any content in the <script> tag.
I don't know what we can do with the let: syntax, but we might even be able to bind something from the +page's <script> to the prop, allowing for an in-place regular callback usage.
| */ | ||
| export let afterClose: () => void = () => {}; | ||
| const onCloseStore = writable(() => {}); |
There was a problem hiding this comment.
- Creating a store that contains a function, initialized with nothing
| </div> | ||
| {/if} | ||
| <div class="-my-6 h-full"> | ||
| <slot onClose={onCloseStore} /> |
There was a problem hiding this comment.
- Passing it as a slot prop, so we can use the
let:directive
| {item.name} | ||
| </button> | ||
| {/each} | ||
| <SlideOver bind:show={showSlideOver} let:onClose> |
There was a problem hiding this comment.
- Get the store as a prop
| on:click={() => { | ||
| slideOverCloseCallback = () => goto("/contact"); | ||
| onClose.set(async () => { |
There was a problem hiding this comment.
- Set it to any value we want inside the callback (a simple function wouldn't be editable otherwise, we can't even use the
$onClose = () => {}syntax)
| leave="ease-in-out duration-500 sm:duration-700" | ||
| leaveFrom="translate-x-0" | ||
| leaveTo="translate-x-full" | ||
| on:outroend={$onCloseStore} |
There was a problem hiding this comment.
- Finally, execute the store's function whenever we want on the parent component, with our custom content!
(Short foron:outroend={() => $onCloseStore()})
Use absolute instead of junky negative margins, fixing every issue it had. Same change for the magnetic arrow. For the hero content (text and right art), define its size to 80svh, which is enough to make it feel centered without having any impact on neither the magnetic arrow nor the background.
- Remove the need for 2 texts for title and subtitle - Improve background sizing - Improve and fix magnetic arrow positioning
- Remove 2 unnecessary goto in favor of "a" tags - Fix logic of detecting active link with the new Inlang paths
MinixBF
approved these changes
Feb 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Main changes
+layout.svelteto avoid duplication and use an inheritance mechanism insteadNote
I'm not a fan of the need to manually tell Inlang which language to use inside
loadfunctions, I don't know if it's a bug.It doesn't alter the behavior of anything, it's just extra verbose for nothing
tailwind-mergeinstead of a dirty manual concatenation within componentsSlideOvernot being dismissable with a click outsideOther changes
eslint .at each commit though)0.xat least)text-prettyon all main<p>s we got for a nicer lookscroll-smoothon navigationgoto()s in favor of direct<a>tags (little follow-up of Convert buttons to links automatically for better accessibility #205)<slot />by props forSection's titleSlideOver:svelte:fragmentsitemap.xml, allowing for a cleaner code and the removal of@types/noderobots.txt, allowing us to use theROOT_URLvariable instead of hardcoding itThings I wish I could fix