Skip to content
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

Organize browse layer code #311

Merged
merged 1 commit into from
Aug 16, 2023
Merged

Organize browse layer code #311

merged 1 commit into from
Aug 16, 2023

Conversation

dabreegster
Copy link
Contributor

We have enough files to organize a bit now.

I'd still like to reduce verbosity / visual noise by removing the "LayerControl" suffix from the filenames, making LayerControls.svelte easier to skim, but that's optional

@Pete-Y-CS
Copy link
Contributor

Pete-Y-CS commented Aug 15, 2023

I was thinking this needed doing so this is very nice! I will say that I think removing layer control from filename is fine if we renamed the directory to layer controls? I guess idk how essential file names are to how you have to call the thing you import/export from that Svelte file. Reason I ask is because I think it's nice to have <XLayerControl> as the svelte component because it describes what it does (in the actual layout/html section of the svelte).

@dabreegster
Copy link
Contributor Author

We can do import XyzLayerControl from "./layers/Xyz.svelte"; the default export can be named however. I'm not too opinionated about it though. https://imfeld.dev/writing/svelte_domless_components is some inspiration for a Svelte component not representing anything in the DOM at all. But in our case, the one component mixes stateful map changes and a bit of DOM

@dabreegster dabreegster merged commit 2471411 into main Aug 16, 2023
2 checks passed
@dabreegster dabreegster deleted the organize_layer_code branch August 16, 2023 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants