Skip to content

Conversation

@pomahtri
Copy link
Contributor

No description provided.

@pomahtri pomahtri requested a review from a team May 19, 2025 04:10
@pomahtri pomahtri self-assigned this May 19, 2025
@pomahtri pomahtri added the 25_1 label May 19, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

The design review point about editors' stylingMode isn't fixed

Let's set the stylingMode:'outline' for editors

Copy link
Contributor

@wdevfx wdevfx May 30, 2025

Choose a reason for hiding this comment

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

❌ Not fixed yet

How it looks (material):
FDB8EEAD-9DBA-4505-AA64-5ED34A2BD783

How it should be (material):
3A9752B6-45FB-40E5-AA8D-C44E5389483C

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed in terms of a separate task -> w1ThR4Sp

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue with data type "date" + editing

Config:

dataSource: [
                        {
                            id: 0,
                            E: new Date('2024-01-01'),
                        }
                    ],
                    keyExpr: 'id',
                    columns: [
                        {
                            dataField: 'E',
                        }
                    ],
                    editing: {
                        mode: 'popup',
                        allowAdding: true,
                        allowUpdating: true,
                        allowDeleting: true,
                    },

Steps:

  1. Edit existing card
  2. Clear the DateEditor value

Expected result
DateEditor without value, we can clear this field value

Actual result
DateEditor set 01/01/1970 value by default, we cannot clear the field value

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed as separate issue: id -> SiNOB53q

Copy link
Contributor

Choose a reason for hiding this comment

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

New functionallity not covered by any type of tests (except some cases)
Let's cover at least happy pathes and discovered bugs cases

Copy link
Contributor

@wdevfx wdevfx May 30, 2025

Choose a reason for hiding this comment

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

🚧 Tests will be implemented in terms of separate task -> LJmmO1Gf

@pomahtri pomahtri closed this May 30, 2025
@pomahtri pomahtri reopened this May 30, 2025
const originalContentReady = simpleFormItem?.editorOptions?.onContentReady;

simpleFormItem.editorOptions = {
stylingMode: 'outline',
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Suggested change
stylingMode: 'outline',
stylingMode: 'outlined',

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed as separate issue: id -> SiNOB53q

return null;
}

const insertChange = changes.find(
Copy link
Contributor

Choose a reason for hiding this comment

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

[For future refactoring]

Let's rename this func, it's find an insert change, don't inserting anything :)


private readonly visible = computed(() => !!this.editingController.editingCard.value);

private readonly customizeItems = computed(() => (item: dxForm.Item): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove computed or leave a comment

Now it's not reactive

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed in terms of a separate card -> aG4dluUl

)?.value ?? null,
onContentReady: (e): void => {
// TODO: refactor
setTimeout(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's clear this timeout in onDispose or sth like this

We cannot leave floating macro tasks by our code rules (in QUnit we have test for it for all components)

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed in terms of a separate task -> iL7V3E1H

Copy link
Contributor

Choose a reason for hiding this comment

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

The delete confirmation dialog always showsa header (by default empty):

3A8FD999-8CDF-4EED-A6EA-30C2A6A4D710

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed in terms of a separate task -> 1BvmPbSB

Copy link
Contributor

Choose a reason for hiding this comment

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

Issue related to pagination

Config

dataSource: new Array(7).fill(null).map((_, idx) => ({
                    id: idx,
                    A: `A_${idx}`,
                    B: `B_${idx}`,
                    C: `C_${idx}`,
                })),
                keyExpr: 'id',
                columns: ['A', 'B', 'C'],
                editing: {
                    allowDeleting: true,
                },

Steps:

  1. Go to the second page
  2. Delete card (there is only one card on the 2nd page)

Expected result
Current page number changed to "1", and we see cards

Actual result
No data screen

Copy link
Contributor

Choose a reason for hiding this comment

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

🚧 will be fixed in terms of separate task -> AYicExDv

@pomahtri pomahtri merged commit d86dd42 into DevExpress:25_1 May 30, 2025
388 of 390 checks passed
@pomahtri pomahtri deleted the card_view/editing/fixes branch May 30, 2025 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants