Development - Latest Merge#75
Conversation
- Add basic HTML structure with sidebar and map container - Include form for inputting workout details with running/cycling options - Add stylesheet defining app layout, colors, and form styling - Include JavaScript for DOM element references and month names - Add images and icons for UI branding and assets - Setup external fonts and defer script loading for performance
- Add for attribute to type label and id to select element for better accessibility - Remove stray character '>' from select element - Add rel="noopener noreferrer" to external Twitter link for security - Replace fixed height with flex-grow on workouts list for better layout flexibility - Add .gitignore file to exclude IDE, node_modules, OS, log, and temp files from repository
feat(mapty): add initial files for workout mapping app
…tion - Implement a ThemeManager class for dark/light mode switching with toggle button UI - Store and apply user theme preference with system theme detection support - Enhance form accessibility with keyboard navigation and dynamic workout type fields - Add Leaflet CSS/JS and initialize map centered on user geolocation - Use Google Maps tiles within Leaflet and add marker with popup for user location - Revamp CSS to use CSS variables for light/dark themes and improve overall styling - Style theme toggle button with hover and focus states for a11y - Update sidebar, workouts list, form, and popup styles for better UI consistency - Add responsive styles for mobile devices and accessibility improvements such as focus outlines - Include smooth transitions, reduced motion support, and high contrast mode compatibility
- Add 'for' attribute and 'id' to distance input for better accessibility - Replace Google Maps tiles with OpenStreetMap tiles in Leaflet map implementation - Correctly get and set theme preference using localStorage in ThemeManager - Refactor DOMContentLoaded event listener for theme initialization and form logic - Fix query selector for cadence input container to correctly find its row - Add braces to geolocation support check for clarity and consistency
- Deleted color property from a:focus selector to improve focus styling - Maintained margin, filter, and transition properties for accessibility - Cleaned up redundant style to streamline CSS rules
feat(ui): add dark mode toggle and integrate Leaflet map with geolocation
…ions - Add ThemeManager class for dark/light mode toggle with persistence - Initialize theme on page load with system preference detection - Improve accessibility with keyboard support on theme toggle button - Implement form field toggling based on workout type selection - Enable keyboard navigation support for workout list items - Initialize Leaflet map centered on user geolocation with marker popup - Show workout input form on map click and focus on distance input - Create popup markers at workout location with custom styling - Clear form inputs and manage cycling/running input visibility dynamically
- Change default theme initialization to use system preference instead of always light - Remove redundant form input toggling logic related to workout type switching - Clean up event listener setup to simplify code and improve maintainability
feat(mapty): implement theme manager and enhance map workout interactions
…storage handling - Implement Workout, Running, and Cycling classes with proper inheritance and methods - Develop App class to manage map, workouts, form interactions, and local storage - Replace previous global variables and functions with encapsulated class logic - Add detailed input validation and user feedback for workout form submissions - Implement map markers and workout list rendering with proper DOM updates - Restore workouts from localStorage with prototype chain re-establishment - Add click-to-move map functionality on workout list interaction - Improve form hiding and clearing logic after submission feat(theme): implement ThemeManager class for dark/light mode with persistence - Initialize theme based on stored preference or system setting - Add toggle button with icon and text updates reflecting current theme - Save user preference to localStorage for session persistence - Listen for system theme changes and auto-update if no user preference - Add keyboard accessibility and smooth scale animation on toggle - Update ARIA attributes for accessibility compliance style(css): enhance styling for theming and workout list responsiveness - Refine CSS variables formatting and consistency for light and dark themes - Improve container overflow and scrolling behavior for sidebar and workouts list - Adjust grid layout of workout details for flexible and wrapped content - Add background color fallback to workout type indicator bar - Improve text wrapping and overflow handling in workout items to prevent clipping
- Remove unused variables and imports - Simplify input validation with positive number checks - Remove debug console logs for cleaner output - Fix localStorage retrieval to restore workout objects properly - Reinstantiate Running and Cycling objects with correct properties - Restore workout descriptions after loading from storage - Adjust render workout marker method call for consistency - Add comments to clarify purpose of critical methods
refactor(app): rewrite workout and app classes with improved map and storage handling
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @srjingole0610, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request integrates the latest development features into the main branch, primarily introducing a new web application named "Mapty". This application allows users to track and visualize their running and cycling workouts on an interactive map, leveraging geolocation and the Leaflet library. It also includes a robust theme manager for dark and light modes, ensuring a personalized user experience, and persists workout data using local storage.
Highlights
- New Mapty Application: Introduction of a new web application for tracking running and cycling workouts on an interactive map.
- Theme Management: Implementation of a dark/light mode toggle with persistence using local storage.
- Workout Tracking: Functionality to record and display running and cycling activities, including distance, duration, pace/speed, and elevation/cadence.
- Geolocation & Map Integration: Utilizes browser geolocation and the Leaflet library to display workouts on an OpenStreetMap.
- Project Setup: Addition of a .gitignore file for better project hygiene.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in pull request comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new workout tracking application, "Mapty". The implementation is well-structured, utilizing modern JavaScript classes and providing a themed UI. However, I've identified several issues related to HTML validity, accessibility, JavaScript logic, and CSS consistency. My feedback includes suggestions to fix a critical HTML error, improve form accessibility, correct a validation bug in the workout creation logic, and enhance the overall robustness and maintainability of the code.
| <div class="form__row"> | ||
| <label class="form__label" for="type">Type</label> | ||
| <select id="type" class="form__input form__input--type"> | ||
| > |
There was a problem hiding this comment.
There is a stray > character inside the <select> element. This is invalid HTML and will be rendered as text on the page. It should be removed to ensure the markup is correct.
| <div class="form__row"> | ||
| <label class="form__label">Duration</label> | ||
| <input | ||
| class="form__input form__input--duration" | ||
| placeholder="min" | ||
| /> | ||
| </div> | ||
| <div class="form__row"> | ||
| <label class="form__label">Cadence</label> | ||
| <input | ||
| class="form__input form__input--cadence" | ||
| placeholder="step/min" | ||
| /> | ||
| </div> | ||
| <div class="form__row form__row--hidden"> | ||
| <label class="form__label">Elev Gain</label> | ||
| <input | ||
| class="form__input form__input--elevation" | ||
| placeholder="meters" | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
For accessibility and proper form functionality, <label> elements should be associated with their corresponding <input> controls. The labels for 'Duration', 'Cadence', and 'Elev Gain' are missing the for attribute, and their inputs are missing id attributes. Adding these will improve the user experience, especially for users of assistive technologies.
<div class="form__row">
<label class="form__label" for="duration">Duration</label>
<input
id="duration"
class="form__input form__input--duration"
placeholder="min"
/>
</div>
<div class="form__row">
<label class="form__label" for="cadence">Cadence</label>
<input
id="cadence"
class="form__input form__input--cadence"
placeholder="step/min"
/>
</div>
<div class="form__row form__row--hidden">
<label class="form__label" for="elevation">Elev Gain</label>
<input
id="elevation"
class="form__input form__input--elevation"
placeholder="meters"
/>
</div>| const validateInputs = (...inputs) => { | ||
| // Check if all inputs are valid positive numbers | ||
| if (!inputs.every(input => Number.isFinite(input) && input > 0)) { | ||
| return { isValid: false, message: 'Inputs must be positive numbers!' }; | ||
| } | ||
| return { isValid: true, message: '' }; | ||
| }; | ||
|
|
||
| // If workout is running, create cycling object | ||
|
|
||
| if (type === 'running') { | ||
| const cadence = +inputCadence.value; | ||
|
|
||
| const validation = validateInputs(distance, duration, cadence); | ||
| if (!validation.isValid) { | ||
| return alert(validation.message); | ||
| } | ||
| workout = new Running( | ||
| [this.#mapEvent.latlng.lat, this.#mapEvent.latlng.lng], | ||
| distance, | ||
| duration, | ||
| cadence, | ||
| ); | ||
| } | ||
|
|
||
| // If workout is cycling, create cycling object | ||
| if (type === 'cycling') { | ||
| const elevation = +inputElevation.value; | ||
|
|
||
| const validation = validateInputs(distance, duration, elevation); | ||
| if (!validation.isValid) { | ||
| return alert(validation.message); | ||
| } | ||
| workout = new Cycling( | ||
| [this.#mapEvent.latlng.lat, this.#mapEvent.latlng.lng], | ||
| distance, | ||
| duration, | ||
| elevation, | ||
| ); | ||
| } |
There was a problem hiding this comment.
This section has a validation bug and can be structured more clearly. The validateInputs function requires all inputs to be positive, which is incorrect for elevation (it can be 0). This prevents logging workouts on flat routes. Additionally, using if...else if is better for mutually exclusive conditions like workout types. Here's a refactoring that fixes the bug and improves the structure.
const validatePositiveNumbers = (...inputs) => {
if (!inputs.every(input => Number.isFinite(input) && input > 0)) {
return { isValid: false, message: 'Inputs must be positive numbers!' };
}
return { isValid: true, message: '' };
};
if (type === 'running') {
const cadence = +inputCadence.value;
const validation = validatePositiveNumbers(distance, duration, cadence);
if (!validation.isValid) {
return alert(validation.message);
}
workout = new Running(
[this.#mapEvent.latlng.lat, this.#mapEvent.latlng.lng],
distance,
duration,
cadence,
);
} else if (type === 'cycling') {
const elevation = +inputElevation.value;
const validation = validatePositiveNumbers(distance, duration);
if (!validation.isValid || !Number.isFinite(elevation)) {
return alert('Distance/duration must be positive, and elevation must be a number.');
}
workout = new Cycling(
[this.#mapEvent.latlng.lat, this.#mapEvent.latlng.lng],
distance,
duration,
elevation,
);
}| // Re-instantiate objects to restore prototype chain | ||
| this.#workouts = data.map(work => { | ||
| let workout; | ||
| if (work.type === 'running') { | ||
| workout = new Running( | ||
| work.coords, | ||
| work.distance, | ||
| work.duration, | ||
| work.cadence, | ||
| ); | ||
| } | ||
| if (work.type === 'cycling') { | ||
| workout = new Cycling( | ||
| work.coords, | ||
| work.distance, | ||
| work.duration, | ||
| work.elevationGain, | ||
| ); | ||
| } | ||
| // Restore original id and date | ||
| workout.id = work.id; | ||
| workout.date = new Date(work.date); | ||
| // Fix description to use the correct stored date | ||
| workout._setDescription(); | ||
| return workout; | ||
| }); |
There was a problem hiding this comment.
When restoring workouts from localStorage, the current implementation creates new Workout objects, which get a new id and date. These are then immediately overwritten with the stored values. This is inefficient and can be confusing.
A better design would be to modify the Workout, Running, and Cycling constructors to optionally accept id and date parameters. This would allow for cleaner object instantiation and avoid redundant property assignments and method calls (_setDescription).
|
|
||
| /* Base body styles */ | ||
| body { | ||
| font-family: 'Inter', -apple-system, BlinkMacSystemFont, sans-serif; |
There was a problem hiding this comment.
The CSS specifies font-family: 'Inter', ..., but the index.html file imports the 'Manrope' font. This mismatch means the intended font is not being used. To fix this, the font-family in the CSS should be updated to 'Manrope' to match the imported font.
| font-family: 'Inter', -apple-system, BlinkMacSystemFont, sans-serif; | |
| font-family: 'Manrope', -apple-system, BlinkMacSystemFont, sans-serif; |
| # Qoder IDE folder | ||
| .qoder/ | ||
|
|
||
| # Node modules (if any) | ||
| node_modules/ | ||
|
|
||
| # OS generated files | ||
| .DS_Store | ||
| .DS_Store? | ||
| ._* | ||
| .Spotlight-V100 | ||
| .Trashes | ||
| ehthumbs.db | ||
| Thumbs.db | ||
|
|
||
| # Logs | ||
| *.log | ||
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
|
|
||
| # IDE files | ||
| .vscode/ | ||
| .idea/ | ||
| *.swp | ||
| *.swo | ||
| *~ | ||
|
|
||
| # Temporary files | ||
| *.tmp | ||
| *.temp No newline at end of file |
There was a problem hiding this comment.
This .gitignore file is a good start. To make it more comprehensive for a typical web project, I suggest adding entries for common build output directories (like dist/ or build/) and environment variable files (e.g., .env). Also, it's a general best practice for text files to end with a newline character.
# Qoder IDE folder
.qoder/
# Node modules (if any)
node_modules/
# Build output
/dist
/build
# Environment variables
.env*
!.env.example
# OS generated files
.DS_Store
.DS_Store?
._*
.Spotlight-V100
.Trashes
ehthumbs.db
Thumbs.db
# Logs
*.log
npm-debug.log*
yarn-debug.log*
yarn-error.log*
# IDE files
.vscode/
.idea/
*.swp
*.swo
*~
# Temporary files
*.tmp
*.temp
| <!-- <li class="workout workout--running" data-id="1234567890"> | ||
| <h2 class="workout__title">Running on April 14</h2> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">🏃♂️</span> | ||
| <span class="workout__value">5.2</span> | ||
| <span class="workout__unit">km</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">⏱</span> | ||
| <span class="workout__value">24</span> | ||
| <span class="workout__unit">min</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">⚡️</span> | ||
| <span class="workout__value">4.6</span> | ||
| <span class="workout__unit">min/km</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">🦶🏼</span> | ||
| <span class="workout__value">178</span> | ||
| <span class="workout__unit">spm</span> | ||
| </div> | ||
| </li> | ||
|
|
||
| <li class="workout workout--cycling" data-id="1234567891"> | ||
| <h2 class="workout__title">Cycling on April 5</h2> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">🚴♀️</span> | ||
| <span class="workout__value">27</span> | ||
| <span class="workout__unit">km</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">⏱</span> | ||
| <span class="workout__value">95</span> | ||
| <span class="workout__unit">min</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">⚡️</span> | ||
| <span class="workout__value">16</span> | ||
| <span class="workout__unit">km/h</span> | ||
| </div> | ||
| <div class="workout__details"> | ||
| <span class="workout__icon">⛰</span> | ||
| <span class="workout__value">223</span> | ||
| <span class="workout__unit">m</span> | ||
| </div> | ||
| </li> --> |
There was a problem hiding this comment.
This large block of commented-out HTML appears to be example data. While useful during development, it adds clutter to the production code. It's best to remove it to keep the codebase clean and maintainable.
| this._loadMap.bind(this), | ||
| function () { | ||
| // Handle geolocation errors by showing user-friendly alert | ||
| alert('Could not get your position ❌'); |
There was a problem hiding this comment.
Using alert() to display errors (here for geolocation and later for form validation) provides a disruptive user experience. Consider implementing a more integrated error display, such as rendering the message within the UI, to create a smoother interaction for the user.
| _hideForm() { | ||
| inputDistance.value = | ||
| inputDuration.value = | ||
| inputCadence.value = | ||
| inputElevation.value = | ||
| ''; | ||
| form.style.display = 'none'; | ||
| form.classList.add('hidden'); | ||
| setTimeout(() => (form.style.display = 'grid'), 1000); | ||
| } |
There was a problem hiding this comment.
The use of form.style.display = 'none' and setTimeout to manage the form's hiding animation is fragile. Setting display: none immediately cancels the CSS transition, and the timeout creates a dependency on the transition duration. A more robust approach is to rely solely on adding/removing the .hidden class, which already handles the animation and visibility.
_hideForm() {
inputDistance.value =
inputDuration.value =
inputCadence.value =
inputElevation.value =
'';
form.classList.add('hidden');
}| .form__btn { | ||
| display: none; | ||
| } |
There was a problem hiding this comment.
The form's submit button is hidden with display: none. While the form can be submitted by pressing Enter, this is not ideal for usability or accessibility, as many users (especially on mobile) rely on a visible button. I recommend making the button visible and styling it to fit the application's theme to provide a clear call to action.
| .form__btn { | |
| display: none; | |
| } | |
| .form__btn { | |
| grid-column: 1 / -1; | |
| background-color: var(--color-primary); | |
| color: var(--color-bg-primary); | |
| border: none; | |
| border-radius: var(--border-radius-sm); | |
| padding: 1rem; | |
| cursor: pointer; | |
| font-size: 1.4rem; | |
| font-weight: 600; | |
| margin-top: 1rem; | |
| } | |
| .form__btn:hover { | |
| background-color: var(--color-secondary); | |
| } |
1 participant
This merge brings the latest development changes into the main branch. It includes new features and improvements
Key updates in this merge:
✅ Feature enhancements
🛠 Code refactoring and cleanup
📄 Updated documentation