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

Prevent creating undefined 'layers-sub-list' when the consecutive layer is not a group #333

Closed
5 tasks
crisner opened this issue Jan 8, 2020 · 2 comments · Fixed by #339
Closed
5 tasks

Comments

@crisner
Copy link
Contributor

crisner commented Jan 8, 2020

Hi, this is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.

If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!

We know that the process of creating a pull request is the biggest barrier for new contributors. This issue is for you 💝

If you have contributed before, consider leaving this one for someone new, and looking through our general help wanted issues. Thanks!

🤔 What you will need to know.

Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.

📋 Step by Step

  • 🙋 Claim this issue: Comment below. If someone else has claimed it, ask if they've opened a pull request already and if they're stuck -- maybe you can help them solve a problem or move it along!

  • 📝 Update the file layersBrowser.js in the leaflet-environmental-layers repository.

See this page for some help in taking your first steps!

Problem

We are creating elements with an id of undefined.
undefinedLayerList

Solution

This section:

if(group !== obj.group) {
this._createGroup(obj);
groupHolder = this._createGroupHolder(obj);
};
if(obj.group) {
groupHolder.appendChild(this._addItem(obj));
} else {
this._addItem(obj);
}

should look this after changes:

      if(obj.group && group !== obj.group) {
        this._createGroup(obj);
        groupHolder = this._createGroupHolder(obj);
      };

      if(groupHolder && obj.group) {
        groupHolder.appendChild(this._addItem(obj));
      } else {
        this._addItem(obj);
      }

The only changes that need to be done are on lines 155 and 160.
This line:

if(group !== obj.group) {

to this:
if(obj.group && group !== obj.group) {

and this line:


to this:
if(groupHolder && obj.group) {

  • 💾 Commit your changes

  • 🔀 Start a Pull Request. There are two ways how you can start a pull request:

  1. If you are familiar with the terminal or would like to learn it, here is a great tutorial on how to send a pull request using the terminal.

  2. You can also edit files directly in your browser and open a pull request from there.

  • 🏁 Done Ask in comments for a review :)

Please keep us updated

💬⏰ - We encourage contributors to be respectful to the community and provide an update within a week of claiming a first-timers-only issue. We're happy to keep it assigned to you as long as you need if you update us with a request for more time or help, but if we don't see any activity a week after you claim it we may reassign it to give someone else a chance. Thank you in advance!

If this happens to you, don't sweat it! Grab another open issue.

Is someone else already working on this?

🔗- We encourage contributors to link to the original issue in their pull request so all users can easily see if someone's already started on it.

👥- If someone seems stuck, offer them some help! Otherwise, take a look at some other issues you can help with. Thanks!

🤔❓ Questions?

Leave a comment below!

@llabake
Copy link
Contributor

llabake commented Jan 8, 2020

Hi, I would like to claim this.

@crisner
Copy link
Contributor Author

crisner commented Jan 8, 2020

Sure, go ahead! 😃

llabake added a commit to llabake/leaflet-environmental-layers that referenced this issue Jan 8, 2020
sagarpreet-chadha added a commit that referenced this issue Mar 25, 2020
* Remove use of hardcoded extents for displaying layers (#332)

* Fix embed code with duplicated text (#340)

* Fix duplication of text in embed code

* Update build file

* Update README.md (#335)

* Update README.md

* Update README.md

* Add check that layers group is not undefined when creating layers-sub-list (#339)

Fixes #333

* Display new items mark in layers menu (#341)

* Create function to check for available layers

* Display badge to show new items

* Save map instance in a variable

* Update readme

* Add contribute button to open infra map

* Add media queries for size 235x300 (#346)

* Add default basemap when undefined (#351)

* Add default basemap when undefined

* Update version number indicating bug-fix

* [WIP] Add layers in include to map by default (#352)

* Add unearthing layer to demo

* Fix bug with removing points from the map on removeLayer

* Add all layers in include param upon map initialization

* bump to v2.1.9

* Implement new menu

* Add new menu dependancies

* Fix styles from overriding default leaflet classes

* Refactor code

* Fix styles for layer group name and layer description

* Add option to disable layer visibility when param.include is true

* Add option to use default layer control

* Fix errors when trying to display layer when out of bounds

* Add missing viewport meta tag

* Move styles to LeafletEnvironmentalLayers.css

* Update documentation

* Add missing dependencies and update build

* Fix error from unearthing layer on page reload

* Set addLayersToMap options from param

* Update documentation

* Update AllLayers.js (#366)

Fixed: Prevent use of the same default tile layer for multiple maps

* Add PLpeopleLayer (#361)

* add PLpeopleLayer to maps

* fix function name to PLpeopleLayer

* fix capital in function name

* make naming consistent for both map examples

* change layer name to PLpeople

* version 2.2.0 and bump LBL to 1.7.0 (#368)

* Feature/minimal mode (#356)

* Create toggle mode control

* Add marker for minimal mode

* Change markers on mode change

* Add condition and marker styles for eonet fires layer

* Add minimal mode content to layers in layercode

* Add minimal mode content to aqicn layer

* Add minimal mode option

* Refactor minimalMode control

* Add mode control to one liner code

* Add documentation to readme

* Fix duplication of layers control, set simple layer control for demo

* Add basic tests

* Update build

* 2.2.0

* Change button names and title

* Update tests for mode control

* Update build

* Add demo with multiple maps in a single page

* Add a check for jQuery and update build

* Bug/fix missing layers in layer menu on refresh (#375)

* Fix 'moveend' event check to affect only when removing layers from map

* Add a check when adding minimal markers to confirm this._map exists

* Update build

* fix conflict

* add parameter display to turn on layers

* check if layer exists before turning it on

* Remove conditions and code responsible for jquery duplication

* Update link to jquery 3.3.1 and remove references to older versions

* Move info.json outside iife and update build

* Install cypress

* published

* Bug/fix layers not removed from map when hash false (#380)

* Add error handling on removing unearthing from map

* Remove redundant code

* Comment out unused baselayer instantiation and update build

* Uncomment baselayers

* Reformat init defaults (#396)

* rewrite init defaults

* remove console.log

* use L.Util.setOptions to set defaults

* Add Cypress

* Add tests to check leaflet left controls

* Add tests for embed prompt and layers menu

* Add child window to prevent iframe conflict on hash change

* Add test to check hash update on map movement

* Move hash test to a seperate file and increase delay

* Set up http server for localhost

* Ignore cypress screenshot and video folders

* Set up cypress in travis

* Oneliner/basic implementation tests (#397)

* testing all the specs

* using babel

* add layer name id to broswer menu

* leaflet not fully loading

* recent tests

* testing error in multiple maps

* rewrite init defaults

* basic one liner tests work

* test shorter transpile files

* emove jquery from gruntfile browserify dist

* use L.Util.setOptions to set defaults

* turn on basic tests again

* disable babel-plugin-transform-object-rest-spread

* clean up

* rebase

* add wisconsin layer back in and fix error

* remove text-table files

* add css file back in

* fix merge errors

* Edit and update documentation for more clarity (#367)

* Add dependencies to documentation

* Add updated documentation to readme

* Add more information

* Edit usage example for layer menu

* Added links to more information

* Update minimal mode documentation

* Add files via upload

* added a layerData folder containing the json file for sea level rise data

Co-authored-by: Renisha Christie. A <29401459+crisner@users.noreply.github.com>
Co-authored-by: Suyash Choudhary <57896905+sssash18@users.noreply.github.com>
Co-authored-by: llabake <lemboyelabake@gmail.com>
Co-authored-by: crisner <christie.reni@gmail.com>
Co-authored-by: Sagarpreet Chadha <chadha.sagarpreet97@gmail.com>
Co-authored-by: jywarren <jeff@unterbahn.com>
Co-authored-by: z-arnott <50159450+z-arnott@users.noreply.github.com>
Co-authored-by: Natalie St Jean <49460529+nstjean@users.noreply.github.com>
Co-authored-by: Natalie <natalie.stjean@gmail.com>
Co-authored-by: Cess <cessmbuguar@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants