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

Use context for all converters (and not only Feature2Mesh) #2196

Merged
merged 2 commits into from
Nov 13, 2023
Merged

Use context for all converters (and not only Feature2Mesh) #2196

merged 2 commits into from
Nov 13, 2023

Conversation

ftoromanoff
Copy link
Contributor

The PR #2180 propose a new approach of Context for the Style, that is used in Feature2Mesh.
2 others converters also use the initial context;

  • Feature2Texture.convert()
  • LabelLayer and Label

This PR propose (based on PR #2180 ) to generalize the use of Context to the other converter.

Changes:

  • export the FeatureContext
  • move it to Style
  • use it for the others converters
  • modify FeatureToolTips plugin to adapt to that change

@ftoromanoff ftoromanoff changed the title Use context for all convert Use context for all converters (and not only Feature2Mesh) Sep 26, 2023
@ftoromanoff ftoromanoff marked this pull request as draft September 26, 2023 14:03
@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch 2 times, most recently from 38bced9 to 06ca7fd Compare October 23, 2023 15:21
@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch 2 times, most recently from cacf832 to a9941ee Compare October 27, 2023 13:32
@Desplandis Desplandis mentioned this pull request Oct 27, 2023
Merged
@jailln
Copy link
Contributor

jailln commented Oct 30, 2023

To remember when reviewing: addess the following comment

@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch 8 times, most recently from d87759d to 6c66c8c Compare November 2, 2023 09:43
@ftoromanoff ftoromanoff marked this pull request as ready for review November 2, 2023 09:44
@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch from 6c66c8c to 2445b87 Compare November 2, 2023 11:12
@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch 3 times, most recently from 0f0a1a5 to 6756148 Compare November 7, 2023 09:19
@ftoromanoff ftoromanoff requested a review from jailln November 7, 2023 09:48
jailln
jailln reviewed Nov 7, 2023
View reviewed changes
src/Converter/Feature2Texture.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Layer/LabelLayer.js Show resolved Hide resolved
src/Main.js Outdated Show resolved Hide resolved
@ftoromanoff ftoromanoff mentioned this pull request Nov 7, 2023
Merged
jailln
jailln reviewed Nov 10, 2023
View reviewed changes
Copy link
Contributor

@jailln jailln left a comment

Choose a reason for hiding this comment

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

thanks for the update. I added some wording suggestions.

src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
src/Core/Style.js Outdated Show resolved Hide resolved
@ftoromanoff ftoromanoff force-pushed the useContextForAllConvert branch from b9e6705 to bb69976 Compare November 13, 2023 09:37
@jailln jailln merged commit 2428d56 into iTowns:master Nov 13, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jailln jailln jailln approved these changes

Assignees

@jailln jailln

@Desplandis Desplandis

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ftoromanoff @jailln @Desplandis