search

Maps4HTML / HTML-Map-Element-UseCases-Requirements

Use cases and requirements for Maps on the Web
https://maps4html.org/HTML-Map-Element-UseCases-Requirements/
Other
22 stars 12 forks source link

More examples completed. #189

Closed NickFitz closed 4 years ago

NickFitz commented 4 years ago

Still a few examples to go, but I'll get these over now so people can check them out (pun intended).

AmeliaBR commented 4 years ago

Whoops. Somehow didn't get back to this when I did reviews last week.

My only concern is adding framework-specific code to examples.css. When @nchan0154 had issues with the TomTom stylesheets code before, she added in local <style> blocks. But if we have to do that for every example page, it's probably better to create a single CSS file with the required rules — but they should still be correctly scoped. I started a dedicated issue for brainstorming: #193

In the meantime, not sure what's the best strategy. I'd like to merge the rest of this.

Malvoz commented 4 years ago

My only concern is adding framework-specific code to examples.css

I'm also responsible for doing that, to make it easier to update styles as we had many duplicate inline styles and style blocks across multiple example pages. So adding a couple more of those now probably wont hurt, especially taking https://github.com/Maps4HTML/HTML-Map-Element-UseCases-Requirements/issues/193#issuecomment-540223696 into consideration.

AmeliaBR commented 4 years ago

to make it easier to update styles as we had many duplicate inline styles and style blocks across multiple example pages

Yeah, I mean, the issue is less about which file to put the CSS in & more about making sure the selectors are sufficiently scoped so there aren't weird interactions & we can easily understand why each rule is there.

Malvoz commented 4 years ago

Oh, I wasn't concerned with which file to put the CSS, but rather why adding inline styles now would be blocking this merge?

I'd like to merge the rest of this.

AmeliaBR commented 4 years ago

OK, I pushed my own "good enough" solution to the TomTom styles issue: I moved the icon rules into their own file, that we can link to everywhere we would link to the TomTom's full stylesheet that includes those rules.

@NickFitz, when checking out the PR, I discovered a mostly commented-out example file. Were you intending to commit that yet? If so, can you tidy it up a bit?

AmeliaBR commented 4 years ago

OK, merging everything. Sorry for the delay. Hoping to find some time/energy to catch up on more substantive reviews, soon.