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.

Sign up for GitHub

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

Jump to bottom

map-extent updates for hidden, disabled, checked, label attributes #255

Merged
merged 1 commit into from
Nov 7, 2023
Merged

map-extent updates for hidden, disabled, checked, label attributes #255

merged 1 commit into from
Nov 7, 2023

Conversation

prushforth
Copy link
Member

No description provided.

@prushforth prushforth mentioned this pull request Nov 6, 2023
Closed
@AliyanH AliyanH linked an issue Nov 6, 2023 that may be closed by this pull request
Update map-extent spec after map-extent custom element development #254
Closed
@AliyanH AliyanH mentioned this pull request Nov 6, 2023
Closed
9 tasks
@prushforth prushforth requested review from AliyanH and yhy0217 November 6, 2023 22:41
yhy0217
yhy0217 reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@yhy0217 yhy0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Just have some ideas about the changes that may make it better:

  1. add the information about the default value of extent.checked when the checked attribute is not supplied, as what we did for label
  2. replace the closing backslash for map-link with the closing tags in index.html and map-1.html
  3. add checked attribute for map-extent in map-1.html and mapml-document.xml

AliyanH
AliyanH reviewed Nov 7, 2023
View reviewed changes
Copy link
Member

@AliyanH AliyanH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

projection web IDL for mapml-viewer is not 'readonly' anymore right?
image

AliyanH
AliyanH reviewed Nov 7, 2023
View reviewed changes
Copy link
Member

@AliyanH AliyanH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be moving our elements up here when they are working as custom elements?

image

AliyanH
AliyanH reviewed Nov 7, 2023
View reviewed changes
spec/index.html Outdated Show resolved Hide resolved
AliyanH
AliyanH reviewed Nov 7, 2023
View reviewed changes
Copy link
Member

@AliyanH AliyanH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something we might be able to add
image

@AliyanH
Copy link
Member

AliyanH commented Nov 7, 2023

Looks good to me! Seems to have everything from the map-extent PR! @prushforth Should #247 be tackled here also? If you think that will interfere with the GeoServer work.

@prushforth
Copy link
Member Author

Should we be moving our elements up here when they are working as custom elements?

One day, as we iterate closer to an HTML Standard PR, we should just treat all the elements as HTML elements, regardless of whether they're new (from MapML) or existing (updates to the spec). That section (3) is due to merging an earlier separate spec that we developed that separated the content we would expect in an HTML document type from the content that we might expect in a MapML document type. The line has blurred a bit with the concept of local and remote content, but I don't think custom / not really implemented is a good way to spec them out.

@prushforth
Copy link
Member Author

Should #247 be tackled here also? If you think that will interfere with the GeoServer work.

It wouldn't interfere, but it's related to <map-feature> fwiw. We will get to it eventually.

@prushforth @yhy0217
Update extent, layer, editors.
b3b52cf 
Add Contributors section, remove authors.
Add `checked` to examples of local and
remote content. Update extent content model
to allow `map-meta` description of zoom and
spatial bounds.

Update resources, examples, checked attribute description

Address @AliyanH comments: projection now read-write.  Remote content
prioritized over local content. map-meta can exist in extent elements.
Fix typo in layer label attribute description.

Co-authored-by: Hanyu Yao <[email protected]>
Co-authored-by: Aliyan Haq <[email protected]>
@prushforth prushforth merged commit 7b24397 into Maps4HTML:gh-pages Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AliyanH AliyanH AliyanH approved these changes

@yhy0217 yhy0217 yhy0217 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update map-extent spec after map-extent custom element development
3 participants
@prushforth @AliyanH @yhy0217