Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Issue 524 - Readonly dropdown column content#530

Merged
Marc-Andre-Rivet merged 10 commits intomasterfrom
524-readonly-dropdown
Aug 5, 2019
Merged

Issue 524 - Readonly dropdown column content#530
Marc-Andre-Rivet merged 10 commits intomasterfrom
524-readonly-dropdown

Conversation

@Marc-Andre-Rivet
Copy link
Copy Markdown
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Aug 2, 2019

Closes #524. Fixes regression introduced by #281.

  • When using a label in-place of dropdown component, make sure to resolve the dropdown label instead of just displaying the value.
  • Update tests so that labels are different from values, making testing for this error easier
  • Additional visual tests

@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 17:39 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 17:40 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 17:41 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 17:57 Inactive
- test both the editable and readonly versions of the dropdown
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 18:26 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 18:38 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 19:16 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 19:30 Inactive
@chriddyp chriddyp temporarily deployed to dash-table-review-pr-530 August 2, 2019 19:50 Inactive
DashTable.getCell(2, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(3, 6).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Wet'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Wet'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Snowy'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

DashTable.getCell(0, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(1, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(2, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
DashTable.getCell(3, 7).within(() => cy.get('.dash-cell-value').should('have.html', 'label: Humid'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

also check the readonly column

)
}
}}
/>)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

visual tests for readonly and editable dropdowns

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Used in conjonction with the updated standalone tests above

@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review August 2, 2019 20:14
'bbb-readonly': {
clearable: true,
options: ['Humid', 'Wet', 'Snowy', 'Tropical Beaches'].map(i => ({
label: `label: ${i}`,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Differentiate between the label and value to catch these differences in the tests.


enum CellType {
Dropdown,
DropdownLabel,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To make it easier to distinguish between the general label case and the dropdown one. Processing is different.


DashTable.getFilterById('bbb').within(() => cy.get('input').should('have.value', 'Tr'));
DashTable.getCellById(0, 'bbb-readonly').within(() => cy.get('.dash-cell-value').should('have.html', 'Tropical Beaches'));
DashTable.getCellById(0, 'bbb-readonly').within(() => cy.get('.dash-cell-value').should('have.html', 'label: Tropical Beaches'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Update tests involving dropdowns so as to expect the label: prefix

Copy link
Copy Markdown
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Works for me - nicely done. 💃

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit 4a10d6a into master Aug 5, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the 524-readonly-dropdown branch September 25, 2019 09:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dropdown columns show their value and not their label when editable=False

3 participants