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

Add formatDate function to calcFunctions #2162

Open
wants to merge 6 commits into
base: beta
Choose a base branch
from

Conversation

TaiKamilla
Copy link
Contributor

@TaiKamilla TaiKamilla commented Dec 12, 2023

Description

Added calc.formatDate method to the text editor
I took the liberty of making some of the parameters optional

Useful links

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (refactor or addition to existing functionality)

Screenshots

Screenshot 2023-12-12 at 18 35 10

Screenshot 2023-12-12 at 18 33 32

Screenshot 2023-12-12 at 18 33 43

Checklist:

( * == Mandatory )

  • * I have set myself as assignee of the pull request
  • * My code follows the style guidelines of this project
  • * Linting does not generate new warnings
  • * I have performed a self-review of my own code
  • * I have put the ticket for review, adding the oort-frontend team to the list of reviewers
  • * I have commented my code, particularly in hard-to-understand areas
  • * I have put JSDoc comment in all required places
  • * My changes generate no new warnings
  • * I have included screenshots describing my changes if relevant
  • * I have selected labels in the Pull Request, according to the changes with code brings

…and-text-widgets # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
@TaiKamilla TaiKamilla added the enhancement New feature or request label Dec 12, 2023
@TaiKamilla TaiKamilla requested a review from a team December 12, 2023 17:17
@TaiKamilla TaiKamilla self-assigned this Dec 12, 2023
@TaiKamilla TaiKamilla marked this pull request as ready for review December 12, 2023 17:34
@TaiKamilla
Copy link
Contributor Author

TaiKamilla commented Dec 12, 2023

@AntoineRelief
the ticket mentions "note that the default format of the date & datetime can certainly be removed if we have that working"
I'm not sure what that is, could you tell me more?

@AntoineRelief
Copy link
Collaborator

@AntoineRelief the ticket mentions "note that the default format of the date & datetime can certainly be removed if we have that working" I'm not sure what that is, could you tell me more?

@TaiKamilla
it was about what I can see in the code
formatDate(date) returns a date with a default format & locale, so all good for me!

@TaiKamilla TaiKamilla closed this Dec 13, 2023
@TaiKamilla TaiKamilla deleted the feat/AB#81470_New-calc-in-template-(-editor-&-summary-card-)-to-format-dates branch December 13, 2023 09:01
@TaiKamilla TaiKamilla restored the feat/AB#81470_New-calc-in-template-(-editor-&-summary-card-)-to-format-dates branch December 13, 2023 09:02
@TaiKamilla TaiKamilla reopened this Dec 13, 2023
@TaiKamilla TaiKamilla force-pushed the feat/AB#81470_New-calc-in-template-(-editor-&-summary-card-)-to-format-dates branch from 1d3b44a to fefb391 Compare December 13, 2023 09:06
@TaiKamilla TaiKamilla changed the base branch from feat/AB#80877_add_logic_to_create_edit_load_page_with_geographic_context to beta December 13, 2023 09:06
…&-summary-card-)-to-format-dates # Conflicts: # libs/shared/src/lib/utils/parser/utils.ts
@RenzoPrats RenzoPrats requested review from RenzoPrats and removed request for a team December 18, 2023 21:03
…&-summary-card-)-to-format-dates # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
Copy link
Contributor

@RenzoPrats RenzoPrats left a comment

Choose a reason for hiding this comment

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

Hi @TaiKamilla, strange issue but it seems not all locale are available, if I put my own local or use the default one(Intl.DateTimeFormat().resolvedOptions().locale) doesn't work in my case(pt-BR), perhaps we could define a default that works when we got error in creating the calc function.

Peek 18-12-2023 18-37

@TaiKamilla
Copy link
Contributor Author

TaiKamilla commented Dec 19, 2023

Hi @TaiKamilla, strange issue but it seems not all locale are available, if I put my own local or use the default one(Intl.DateTimeFormat().resolvedOptions().locale) doesn't work in my case(pt-BR), perhaps we could define a default that works when we got error in creating the calc function.

This is due to the fact that not all locales are imported by default, and it might not be a great idea to import them all, because size

Adding this will make Spanish and Portuguese (and the others like pt-BR) work

import localeEs from '@angular/common/locales/es';
import localePt from '@angular/common/locales/pt';

registerLocaleData(localeEs);
registerLocaleData(localePt);
image

Maybe we get a selection? or we can leave it like this
The system apparently has fr-FR and en-GB imported by the application somewhere else and that is why they work on those

Maybe some feedback on this, @AntoineRelief?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants