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 #4352 - ListOperation modal no longer showing when ellipsis is clicked #4377

Merged
merged 1 commit into from
May 16, 2022

Conversation

promatik
Copy link
Contributor

@promatik promatik commented May 15, 2022

Fix for #4352

WHY

BEFORE - What was wrong? What was happening before this PR?

Ellipsis with row details were not working.

AFTER - What is happening after this PR?

Details are working again with the modal 🙌

HOW

How did you achieve that, in technical terms?

There was a conflict with a data-responsive attribute on table 🤦‍♂️
By changing data-responsive to data-responsive-table the conflict is solved.

Is it a breaking change?

No.
The big issue here is that people need to republish the assets to get everything working properly.
The good news is, this fix will make the modal work for everybody with no publish, the publish will fix a minor issue, the changes on the CSS just set some stuff for non responsive tables (few people use).

That said, the worst case here is devs who use non responsive table, will now get the strange rows in the beginning and end of the table, that were there in the v5 lunch (fixed by that PR in 5.0.14);

image


Any way, tested everything again with all the variables, let's hope this is done for good, with no issues now 🙌

@tabacitu
Copy link
Member

Thank you @promatik - merged. In hindsight... we probably should have gone with has-responsive-table... it would've been both unique and consistent with the other attributes... Oh well..

@promatik
Copy link
Contributor Author

In this case "is" would make more sense...
<table data-is-responsive="true"

@tabacitu
Copy link
Member

Ugh even better 😅 Eh... it is what it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants