Add minimize button to infowindow

[huggla]

I created a minimize button for the infowindow (like I've previously did for the popup #1949).

I'm not sure if or how the tabindex should be changed though.

[johnnyblasta]

You can read about tabindex here but 0 is a good value here. Negative value means the button cannot be focused on.

[tonnyandersson]

Seems to minimize a little too much, resulting in button covering text.

[huggla]

I can't reproduce, but it was probably caused by a mix of px/em/rem in padding/margin. Can you check if I managed to fix it?

[tonnyandersson]

Looks better now. But I noticed something else. If you:

1. Click on an object to open the infowindow
2. Minimize the infowindow
3. Close the infowindow
4. Click on a new object to open the infowindow

...you will notice that the infowindow opens in minimized mode. I don't think this should be the default behavior.

[huggla]

I've fixed so infowindow doesn't remained minimized after clicking close.

[johnnyblasta]

Why is the icon different compared to the popup in #1949

[huggla]

Why is the icon different compared to the popup in #1949

The buttons were implemented differently in popup vs infowindow. In popup I could use a simple underscore as button, but infowindow needed an svg icon. As there was no minimize icon available, I chose the pinch icon.

[tonnyandersson]

LGTM.

But I think we should discuss the icons. It makes no sense to have different icon styles in similar elements. Not necessarily within the scope of this particular issue, but the discussion must be had.

[tonnyandersson]

So, I had a look at the code and I think that parts of the code could be written in a different way, more akin to how similar stuff in Origo is written (yes, I'm aware of the fact that the Origo code isn't 100% consistent in this regard, but still...).

Specifically, I'm referring to the resetInfowindow() and minimizeInfowindow() functions. Typically, the hidden class would be used rather than "manually" setting different style parameters. Sort of like this:

function resetInfowindow() {
  urvalListContainer.classList.remove('hidden');
  listContainer.classList.remove('hidden');
  exportContainer.classList.remove('hidden');
  groupFooterContainer.classList.remove('hidden');
  minimizeButton.setIcon(initialIcon);
  minimizeButton.setState('initial');
}

function minimizeInfowindow() {
  urvalListContainer.classList.add('hidden');
  listContainer.classList.add('hidden');
  exportContainer.classList.add('hidden');
  groupFooterContainer.classList.add('hidden');
  minimizeButton.setIcon(hiddenIcon);
  minimizeButton.setState('hidden');
}

function toggleInfowindow() {
  if (minimizeButton.getState() === 'hidden') {
    resetInfowindow();
  } else {
    minimizeInfowindow();
  }
}

There's probably an even more elegant way to do it, but I guess you get the idea.

NOT saying you must change this now, just stating my very own opinion in the matter. Perhaps a topic to discuss in the next meeting?

[huggla]

I've now made changes to the code according to our discussion, but I'm unsure of the correct placement of the const definitions below (Lint complains). Feedback appreciated.

const initialMinimizeButtonIcon = '#ic_close_fullscreen_24px';
const hiddenMinimizeButtonIcon = '#ic_open_in_full_24px';
const minimizeButton = createMinimizeButton();

[tonnyandersson]

I would have declared all three at the top (same as now) with let just like the other variables there, and then set them at the appropriate places.

I think the minimizeButton should be set in the render() function, before it is appended to the urvalContainer. The two icon variables can be set in the init() function.

[huggla]

I would have declared all three at the top (same as now) with let just like the other variables there, and then set them at the appropriate places.

I've been taught to never use a variable for something that can be a constant, but I'll do it like that if that's how you prefer it.

[huggla]

The two icon variables can be set in the init() function.

There are two blocks of variable definitions in init(), separated by an empty line, I've put the icons in the second block.

When and where should we put empty lines in the code?

[huggla]

I've updated my code.