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

Datatables Loading Icon #1406

Merged
merged 3 commits into from
Sep 15, 2023
Merged

Datatables Loading Icon #1406

merged 3 commits into from
Sep 15, 2023

Conversation

JVickery-TBS
Copy link
Contributor

feat(templates): loading icon for datatables;

  • Added html loading icon and behavior for datatables.

- Added html loading icon and behavior for datatables.
@wardi
Copy link
Member

wardi commented Sep 14, 2023

if you use webassets for these files instead then then they will be given cache-busting urls automatically so we don't have to do the silly hack like we have for the canada1.css file

@wardi
Copy link
Member

wardi commented Sep 14, 2023

Are you adding this to the ckanext-canada extension because of a BS/WET compatibility issue that would be more complicated to fix in ckan?

@JVickery-TBS
Copy link
Contributor Author

@wardi I did it in the Canada Ext because we had to change the colors, and I personally wanted to reposition the spinner. I also wanted to have the spinner visible on first initialization of the datatable.

I can fix this upstream however if you would want. I just assumed that the behavior was intended, as the processing setting of the datatables was turned off and there was no data attribute coded in to control this feature. So thought it was on purpose.

open-data/ckan@b08efcb <- the note in the commit is Disable processing spinner - didn't automatically disappear. Which I have never found to be the case.

We would still probably want to keep the code in this PR for the positioning of it. But in the upstream, I would be adding new datat attributes to the datatables_view ckan js module to allow for the spinner to be enabled, and a preInit spinner as well.

@wardi
Copy link
Member

wardi commented Sep 14, 2023

Sure, this makes sense here if it's for matching our theme.

@JVickery-TBS
Copy link
Contributor Author

@wardi okay it is using webassets now. I think I did it right-ish

@wardi
Copy link
Member

wardi commented Sep 15, 2023

try moving webassets to an assets folder so that they aren't also served as public files

- Moved datatables assets to `assets` directory.
@JVickery-TBS
Copy link
Contributor Author

@wardi okay they are now in an assets directory

@JVickery-TBS JVickery-TBS merged commit fd9a82c into canada-v2.9 Sep 15, 2023
2 checks passed
@JVickery-TBS JVickery-TBS deleted the feature/datatables-loader branch September 15, 2023 15:12
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