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.

Sign up for GitHub

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

Jump to bottom

Setup Leaflet and React Leaflet #11

Merged
merged 8 commits into from
Mar 19, 2023
Merged

Setup Leaflet and React Leaflet #11

merged 8 commits into from
Mar 19, 2023

Conversation

MacAndersonUche
Copy link
Collaborator

@MacAndersonUche MacAndersonUche commented Mar 16, 2023 •
edited by toreylittlefield
Loading

  • Install react-leaflet and leaflet in nextjs app.
  • Creates a simple working example of a map

References Issue: #8

@MacAndersonUche MacAndersonUche self-assigned this Mar 16, 2023
@MacAndersonUche MacAndersonUche linked an issue Mar 16, 2023 that may be closed by this pull request
Setup Leaflet and React Leaflet #8
Closed
ranegray
ranegray previously approved these changes Mar 16, 2023
View reviewed changes
@toreylittlefield
Copy link
Collaborator

toreylittlefield commented Mar 17, 2023 •
edited
Loading

@MacAndersonUche can you reference the GH issue here? You obviously didn't read the whole ticket? Did chatgpt do this for you?

@ranegray ranegray mentioned this pull request Mar 17, 2023
Closed
@ranegray ranegray self-requested a review March 17, 2023 16:52
@ranegray
Copy link
Collaborator

ranegray commented Mar 17, 2023 •
edited
Loading

Added Map.tsx to pages folder as a component. I'm having trouble with Typescript errors because they aren't recognizing the react-leaflet component attributes. Tried installing @types/react-leaflet as per the docs but alas didn't fix the red squiggles. The map will need to be centered still and markers will need to be ironed out. Let me know if there is anything glaringly breaking.

From Setup Leaflet and React Leaflet #8:
In the nextjs app we'll need leaflet.js and react-leaflet installed and up in working.
When the nextjs application loads the default center of the map should be for now which should display a bit of Bali, ID on the map.

const defaultCenter = [-8.7445,115.1820];
const defaultZoom = 11;

@ranegray ranegray requested a review from Netdreion March 17, 2023 16:58
@ranegray ranegray assigned ranegray and unassigned MacAndersonUche Mar 17, 2023
toreylittlefield
toreylittlefield reviewed Mar 18, 2023
View reviewed changes
pages/index.tsx Outdated
Comment on lines 6 to 8
const MapWithNoSSR = dynamic(() => import('./Map'), {
ssr: false,
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can skip the dynamic import for now and keep it simple. Just import and render the component. Is this working for you? It wasn't for me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that it is not working for me

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: I read instructions on react-leaflet and nextjs ssr. Got it. This looks correct with the dynamic import like this for now. If we use SSR that'll be useful.

@toreylittlefield
Copy link
Collaborator

toreylittlefield commented Mar 18, 2023 •
edited
Loading

For Map.jsx you can move it of the pages dir which is reserved in nextjs for pages (don't worry about it) and create a folder in the root of our project call lib. Inside lib create a components folder. Put Map.jsx inside components for now 👍

So importing Map should have the path @/lib/components/Map or ../lib/components/Map

Nextjs file structure is just a little different than React. Sorry I forgot to tell you.

@toreylittlefield
Copy link
Collaborator

@ranegray I took a look at this and I apologize it is working for me. It's just I couldn't see the map. Can you adjust the style to make the map fit the entire viewport?

toreylittlefield
toreylittlefield reviewed Mar 19, 2023
View reviewed changes
next.config.js
Comment on lines +7 to +19
webpack: (config) => {
config.plugins.push(
new CopyPlugin({
patterns: [
{
from: 'node_modules/leaflet/dist/images',
to: path.resolve(__dirname, 'public', 'leaflet', 'images')
},
],
}),
)
return config
}
Copy link
Collaborator

@toreylittlefield toreylittlefield Mar 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranegray @MacAndersonUche why do we need this? Looks like it's copying over the layers, and marker images to over public dir.

We can customize the markers and stuff in the future 👍
Found the answer here: https://jan-mueller.at/blog/react-leaflet/

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left this config in. Not sure if you wanted me to remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes looks like we need it for now 👍

toreylittlefield
toreylittlefield reviewed Mar 19, 2023
View reviewed changes
pages/Map.jsx Outdated Show resolved Hide resolved
toreylittlefield
toreylittlefield reviewed Mar 19, 2023
View reviewed changes
pages/Map.jsx Outdated Show resolved Hide resolved
toreylittlefield
toreylittlefield reviewed Mar 19, 2023
View reviewed changes
pages/Map.jsx Outdated
return null;
}

export default function Map() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use const instead of function just preference 👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to const in newest commit. export default at bottom of file.

@ranegray
Copy link
Collaborator

Newest commit. Added full viewport map, added lib/component file structure. Changed naming convention of function to arrow sytanx with default export at bottom of Maps.jsx. Removed any imports or functions that weren't being used.

Clean slate ready for whatever.

toreylittlefield
toreylittlefield reviewed Mar 19, 2023
View reviewed changes
lib/Components/Map.jsx Outdated
@@ -17,7 +11,7 @@ export default function Map() {
<MapContainer
center={center}
zoom={12}
style={{ height: "50%", width: "100%" }}
style={{ height: "100vh", width: "100vw" }}
Copy link
Collaborator

@toreylittlefield toreylittlefield Mar 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranegray for width you don't need to specify the viewport units as the cause problems👍 100% is good. I pushed up the change for you

toreylittlefield
toreylittlefield approved these changes Mar 19, 2023
View reviewed changes
Copy link
Collaborator

@toreylittlefield toreylittlefield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranegray well done 🚀

@toreylittlefield toreylittlefield merged commit 90fe552 into main Mar 19, 2023
@ranegray ranegray deleted the mu/issue-8-add-leftlet branch March 21, 2023 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@toreylittlefield toreylittlefield toreylittlefield approved these changes

@ranegray ranegray Awaiting requested review from ranegray

@Netdreion Netdreion Awaiting requested review from Netdreion

Assignees

@ranegray ranegray

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Setup Leaflet and React Leaflet
3 participants
@MacAndersonUche @toreylittlefield @ranegray