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

[#1156] Add email and phonenumber to map and location cards #495

Merged

Conversation

vaszig
Copy link
Contributor

@vaszig vaszig commented Feb 23, 2023

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2023

Codecov Report

Merging #495 (3b1fed2) into develop (9bbbfb0) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##           develop     #495      +/-   ##
===========================================
- Coverage    96.50%   96.50%   -0.01%     
===========================================
  Files          514      514              
  Lines        18539    18562      +23     
===========================================
+ Hits         17891    17913      +22     
- Misses         648      649       +1     
Impacted Files Coverage Δ
...rc/open_inwoner/pdc/tests/test_product_location.py 100.00% <ø> (ø)
src/open_inwoner/pdc/models/mixins.py 87.80% <75.00%> (-1.09%) ⬇️
src/open_inwoner/plans/forms.py 100.00% <0.00%> (ø)
src/open_inwoner/plans/tests/test_views.py 100.00% <0.00%> (ø)
src/open_inwoner/plans/views.py 97.87% <0.00%> (+0.03%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@Bartvaderkin Bartvaderkin left a comment

Choose a reason for hiding this comment

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

In the map bubble, I think we'd want each property on it's own line like on the detail page? It is unusual to concatenate them together.

src/open_inwoner/js/components/map/index.js Outdated Show resolved Hide resolved
@vaszig vaszig changed the title [#1156] Feature/add email and phonenumber to map and location cards [#1156] Add email and phonenumber to map and location cards Feb 24, 2023
@vaszig vaszig requested review from Bartvaderkin and removed request for Bartvaderkin February 24, 2023 13:57
@vaszig
Copy link
Contributor Author

vaszig commented Mar 1, 2023

I had to specify the version of babel-loader package since it was updated automatically to the latest version (not compatible with the CI node version).

image

@vaszig vaszig requested review from Bartvaderkin and removed request for Bartvaderkin March 2, 2023 08:43
@vaszig vaszig requested a review from Bartvaderkin March 2, 2023 13:16
@alextreme
Copy link
Member

Jiro can do a double-check first

@vaszig vaszig requested review from jiromaykin and removed request for jiromaykin March 3, 2023 07:58
@jiromaykin
Copy link
Contributor

I had to specify the version of babel-loader package since it was updated automatically to the latest version (not compatible with the CI node version).

Which CI node-version are we using? Should we keep NodeJS to the latest version or not? I don't know much about this yet :-) so I wonder how we will keep track of CI node and change Babel-parser to latest again?

Copy link
Contributor

@jiromaykin jiromaykin left a comment

Choose a reason for hiding this comment

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

Looks all good to me now ,when viewed in browser.

@vaszig
Copy link
Contributor Author

vaszig commented Mar 3, 2023

I had to specify the version of babel-loader package since it was updated automatically to the latest version (not compatible with the CI node version).

Which CI node-version are we using? Should we keep NodeJS to the latest version or not? I don't know much about this yet :-) so I wonder how we will keep track of CI node and change Babel-parser to latest again?

In CI we are using node 12.22.12, which is not compatible with the latest version of babel-loader. I am not an expert concerning all these, but I know that it's better to pin a problematic package instead of upgrading node (which may cause problems to several packages then).

@alextreme alextreme merged commit a54aa30 into develop Mar 3, 2023
@alextreme alextreme deleted the feature/1156-add-email-phonenumber-to-location-cards branch March 3, 2023 10:39
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

Successfully merging this pull request may close these issues.

5 participants