Skip to content
This repository was archived by the owner on Jun 4, 2024. It is now read-only.

Issue 524 - Readonly dropdown column content #530

Merged
merged 10 commits into from
Aug 5, 2019

Conversation

Marc-Andre-Rivet
Copy link
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
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
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
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
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
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
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
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.

@@ -26,6 +26,7 @@ const mapRow = R.addIndex<IColumn, JSX.Element>(R.map);

enum CellType {
Dropdown,
DropdownLabel,
Copy link
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.

@@ -101,7 +101,7 @@ describe('filter', () => {
DashTable.getFilterById('ccc').click();

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
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
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