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

Fix: [DEV-9001] date format for table #1666

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

Conversation

Atash3000
Copy link
Collaborator

[Replace With Ticket Number]

Testing Steps

Open Bar/Line/Combo charts.
Switch to Date time or Date scale
Enter date format for text field "Data Table date display format" which updates only data table if entered.
The data table date format should be changed entered date format.

Self Review

  • I have added testing steps for reviewers
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing unit tests are passing

Screenshots (if applicable)

Additional Notes

@Atash3000 Atash3000 added this to the 4.24.12 milestone Nov 11, 2024
@joshlacey joshlacey changed the title [DEV-9001] Fix date format for table FIX [DEV-9001] date format for table Nov 12, 2024

if (isDateTime && !config.table.dateDisplayFormat) {
cellValue = formatDate(config.xAxis?.dateDisplayFormat, parseDate(config.xAxis?.dateDisplayFormat, labelValue))
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

make this more readable:

if (isDateTime) { const dateDisplayFormat = config.table?.dateDisplayFormat || config.xAxis?.dateDisplayFormat; cellValue = formatDate(dateDisplayFormat, parseDate(dateDisplayFormat, labelValue) }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed!

Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment was for this and the if statement above it. You can handle both with these couple lines.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can you explain more please? I did not understood the issue here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

code is readable. should we remove if statement or add alll logic here : if ((column === config.xAxis?.dataKey && type === 'date') || (type === 'date-time' && type === 'continuous')) {

@Atash3000 Atash3000 changed the title FIX [DEV-9001] date format for table Fix; [DEV-9001] date format for table Nov 26, 2024
@Atash3000 Atash3000 changed the title Fix; [DEV-9001] date format for table Fix: [DEV-9001] date format for table Nov 26, 2024
: labelValue
cellValue =
config.xAxis?.type === 'continuous' ? formatNumber(runtimeData[row][column], 'bottom', false, config) : labelValue
let { type, dateDisplayFormat } = config.xAxis || {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

cellValue =
config.xAxis?.type === 'continuous' ? formatNumber(runtimeData[row][column], 'bottom', false, config) : labelValue
let { type, dateDisplayFormat } = config.xAxis || {}
let dateFormat = config.table?.dateDisplayFormat || dateDisplayFormat
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

cellValue = formatNumber(runtimeData[row][column], 'bottom', false, config)
} else {
cellValue = labelValue
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

this logic works otherwise, just change those above consts since those aren't variable.

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.

3 participants