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.

Sign up for GitHub

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

Jump to bottom

ELEMENTS-1175: fix checkbox behavior on nuxeo-data-table after polymer update #283

Closed
wants to merge 1 commit into from
Closed

ELEMENTS-1175: fix checkbox behavior on nuxeo-data-table after polymer update #283

wants to merge 1 commit into from

Conversation

semisse
Copy link
Collaborator

@semisse semisse commented Sep 2, 2020

No description provided.

@semisse semisse requested a review from a team as a code owner September 2, 2020 16:41
@nuxeojenkins
Copy link
Contributor

View issue in JIRA: ELEMENTS-1175: nuxeo-checkmark is not clickable on nuxeo-data-table

richardsd
richardsd previously approved these changes Sep 2, 2020
View reviewed changes
nmpcunha
nmpcunha reviewed Sep 2, 2020
View reviewed changes
Copy link

@nmpcunha nmpcunha left a comment

Choose a reason for hiding this comment

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

My memory won't probably serve me well, since it was long time ago.
But, in any case, I think we've tried something similar at the time and we broke some other usage of data-table (not sure with or without the checkmark).

I would recommend you to take some time to test several usages of it 😉

@semisse semisse removed the request for review from RSalem07 September 2, 2020 17:02
mnixo
mnixo previously approved these changes Sep 3, 2020
View reviewed changes
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-elements-pr-283 here

andreacornaglia
andreacornaglia reviewed Sep 7, 2020
View reviewed changes
Copy link
Contributor

@andreacornaglia andreacornaglia left a comment

Choose a reason for hiding this comment

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

should we change the commit message to reflect the main goal of this fix? something like "fix checkbox behavior on nuxeo-data-table after polymer update"?

@semisse
Copy link
Collaborator Author

semisse commented Sep 7, 2020

Yes we can, @andreacornaglia. Your suggestion is clearer. Thanks!

@semisse semisse dismissed stale reviews from mnixo and richardsd via 1cd8af2 September 7, 2020 14:43
@semisse semisse force-pushed the fix-ELEMENTS-1175-removeDispatchEventFromNuxeoDataTable branch from 250c0da to 1cd8af2 Compare September 7, 2020 14:43
@semisse semisse changed the title ELEMENTS-1175: remove dispatch event from nuxeo-data-table ELEMENTS-1175: fix checkbox behavior on nuxeo-data-table after polymer update Sep 7, 2020
@nuxeo-web-ui-jx-bot
Copy link
Contributor

⭐ PR built and available in a preview environment nuxeo-nuxeo-elements-pr-283 here

nmpcunha
nmpcunha reviewed Sep 8, 2020
View reviewed changes
Copy link

@nmpcunha nmpcunha left a comment •
edited

Choose a reason for hiding this comment

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

Looking to the existing JIRAS, it seems that the upgrade broke our CI, so it would be a great start: to ensure that our tests are passing.
But, in other to achieve that we'll need changes on WebUI side, right? The dependency is still "locked" there (https://github.com/nuxeo/nuxeo-web-ui/blob/master/package.json#L165). 🤷🏼‍♂️

Not sure if you done it locally, but we'll need to run the pipeline with changes on both repos. 😉

@semisse
Copy link
Collaborator Author

semisse commented Sep 8, 2020 •
edited

Build ok with polymer dependency unlocked 🎉
(cc @nmpcunha )

nmpcunha
nmpcunha reviewed Sep 16, 2020
View reviewed changes
Copy link

@nmpcunha nmpcunha left a comment •
edited

Choose a reason for hiding this comment

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

Glad to know that it works! 😉

Maybe it is important to add this to the upgrade notes, since we might break compatibility if someone is using this element and relying on that event!

@semisse
Copy link
Collaborator Author

semisse commented Nov 27, 2020

the solution suggested here breaks the multiselection on nuxeo-datatable. We need to spend more time investigating this problem.

@semisse semisse closed this Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmpcunha nmpcunha nmpcunha left review comments

@richardsd richardsd richardsd approved these changes

@mnixo mnixo mnixo approved these changes

@nelsonsilva nelsonsilva Awaiting requested review from nelsonsilva

@andreacornaglia andreacornaglia Awaiting requested review from andreacornaglia

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@semisse @nuxeojenkins @nuxeo-web-ui-jx-bot @richardsd @mnixo @andreacornaglia @nmpcunha