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

Elevation chart not responsive on detachedview=false #6

Closed
serako opened this issue May 5, 2019 · 8 comments
Closed

Elevation chart not responsive on detachedview=false #6

serako opened this issue May 5, 2019 · 8 comments

Comments

@serako
Copy link

serako commented May 5, 2019

Hi,
I use your plugins in detachedview=false, but when the weight screen is small, the elevation is not reduce on width.

I do this in line 73

if ( !opts.detachedView && opts.responsiveView) {
  var respWidth = map._container.clientWidth;
  opts.width = opts.width - 20 > respWidth ? respWidth - 20 : opts.width;
}

But it's not very nice because if you rotate your phone, the elevation graph don't adjust on the new width.

@Raruto
Copy link
Owner

Raruto commented May 5, 2019

Unfortunately, even in the original library, a shared solution had not yet been reached... (see here for more details)

your idea might work, but we should also have to change this function so that the control size will updated correctly (even when the control is "within" the map).

    this._map.on('resize', function(e) {
      var mapDiv = this._map.getContainer();
      var eleDiv = document.querySelector(this.options.elevationDiv); //eg. #elevation-div

      var newWidth = eleDiv.offsetWidth;

      if (newWidth <= 0) return;

      this.options.width = newWidth;

      eleDiv.innerHTML = ""; // analogous to: map.removeControl(elevationControl);

      var container = this.onAdd(this._map);
      container.classList.add("leaflet-control");

      if (this.options.detachedView) {
        eleDiv.appendChild(container); // analogous to: elevationControl.addTo(map); 
      } else {
        // if (this.options.detachedView == false)
        this.addTo(this._map); // FIXME: the control is always appended to the map!
      }
    }, this);

@serako
Copy link
Author

serako commented May 6, 2019

I do :
line 74:

  if ( !opts.detachedView && opts.responsiveView) {
      opts._maxWidth = opts._maxWidth > opts.width ? opts._maxWidth : opts.width
      var containerWidth = map._container.clientWidth;
      opts.width = opts._maxWidth > containerWidth ? containerWidth - 30 : opts.width;
    }

and after on new line 193

    this._map.on('resize', function(e) {
      if ( !this.options.detachedView && this.options.responsiveView) {

        var eleDiv = this._container
        map.removeControl(this._container);

        var container = this.onAdd(this._map);
        eleDiv.classList.add("leaflet-control");

        eleDiv.appendChild(this._container);
        this.addTo(this._map);

      }

      this._resetView;

    }, this);

It s nice for responsive, but i think i dont remove the good container because after change my width screen i loop on a fantom object.
or this is not good : eleDiv.appendChild(this._container);

@Raruto
Copy link
Owner

Raruto commented May 6, 2019

I think the control logic must be separated if we are using the default "leaflet-container" or a generic "html-container" (eg. #elevation-div)

this._map.on('resize', function(e) {

    if (this.options.responsiveView) {

        if ( this.options.detachedView) {
          ...
            eleDiv.innerHTML = "";
            eleDiv.appendChild(container);
          ...
        } else {
          ...
            this._map.removeControl(this._container); // NB. "this._map" not just "map"
            this.addTo(this._map); // in my old tests this was a problematic part...
          ...
        }

    }

}

@serako
Copy link
Author

serako commented May 8, 2019

same problem : i loop.
first resize i loop x2, second resize i loop x4 ...

this.addTo(this._map); // in my old tests this was a problematic part...

Perhaps function addTo() create a new instance?
i try lot of test but i'm not a confirmed javascript programmer!

@Raruto
Copy link
Owner

Raruto commented May 8, 2019

Take a look at the changes just made in the develop branch and let me know...

@serako
Copy link
Author

serako commented May 9, 2019

on line 66, separate logic like you prefer:

if (opts.responsiveView) {
      
  if (opts.detachedView) {
        
    var offsetWi = document.querySelector(opts.elevationDiv).offsetWidth;
    var offsetHe = document.querySelector(opts.elevationDiv).offsetHeight;
    opts.width = offsetWi > 0 ? offsetWi : opts.width;
    opts.height = (offsetHe - 20) > 0 ? offsetHe - 20 : opts.height - 20;
        
  } else {
        
    opts._maxWidth = opts._maxWidth > opts.width ? opts._maxWidth : opts.width
    var containerWidth = map._container.clientWidth;
    opts.width = opts._maxWidth > containerWidth ? containerWidth - 30 : opts.width;
        
  }
}

and in function addData() in new line 866

this._attachMapListeners();

This because i use directely L.GPX for add my polyline, and after el.addData( x.line);
I use that because on my map i have lot of gpx, and i can switch the elevation curve if i select a gpx.

Now, it's work fine. but i don't test with opts.detachedView = true and addGPXFile and addGeoJSONFile

thank you for your help!

@Raruto
Copy link
Owner

Raruto commented May 9, 2019

addGPXFile: function(url) {
  ...
    this._attachMapListeners(); // handle chart resizes
  ...
}

maybe that wasn't the right place...
I moved / renamed the _attachMapListeners function into the _resizeChart (is that okay with you?)

@serako
Copy link
Author

serako commented May 10, 2019

I try, it's OK.
I don't try with addGPXFile, just with addData.

@Raruto Raruto closed this as completed in 82d4ed6 May 10, 2019
@Raruto Raruto changed the title elevation not responsive on detachedview=false Elevation chart not responsive on detachedview=false Sep 24, 2019
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

No branches or pull requests

2 participants