8000 activeadmin v2.10.x compatibility by gigorok · Pull Request #73 · activeadmin-plugins/active_admin_datetimepicker · GitHub
[go: up one dir, main page]
Skip to content

activeadmin v2.10.x compatibility #73

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
Closed
wants to merge 1 commit into from
Closed

activeadmin v2.10.x compatibility #73

wants to merge 1 commit into from

Conversation

gigorok
Copy link
Collaborator
@gigorok gigorok commented Feb 12, 2022 •
edited
Loading

the latest active_admin_datetimepicker v0.7.4 is not compatible with activeadmin v2.10.x, but compatible with v2.11.0

@sunny
Copy link
Contributor
sunny commented Feb 15, 2022 •
edited
Loading

This is the Sass error I am getting using active_admin 2.10.0:

Error: Undefined variable: "$filter-field-seperator-width".
        on line 7:16 of ../../.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/active_admin_datetimepicker-0.7.4/app/assets/stylesheets/active_admin_datetimepicker.scss
        from line 16:9 of app/assets/stylesheets/active_admin.scss
>>         width: $filter-field-seperator-width;

My line 16 of app/assets/stylesheets/active_admin.scss is:

@import "active_admin_datetimepicker";

The issue is fixed for me by changing this line into the following:

$filter-field-seperator-width: 12px;
$sidebar-inner-content-width: $sidebar-width - ($section-padding * 2);
$side-by-side-filter-input-width:
  ($sidebar-inner-content-width * 0.5) -
  ($text-input-horizontal-padding * 2) -
  $filter-field-seperator-width;
@import "active_admin_datetimepicker";

The issue may come from the fact that these variables are not accessible from the main scope anymore, as they are imported inside a @meda screen scope. Perhaps we should simply redefine them at the top of active_admin_datepicker.

@gigorok
Copy link
Collaborator Author
gigorok commented Feb 15, 2022

@sunny thanks for the workaround. looks like needs to move stylesheets in active_admin_datetimepicker to @media screen as well.

@gigorok
Copy link
Collaborator Author
gigorok commented Feb 15, 2022

@workgena @Fivell what about moving away from travis to github actions? cause now specs are not running

@sunny
Copy link
Contributor
sunny commented Feb 15, 2022 •
edited
Loading

looks like needs to move stylesheets in active_admin_datetimepicker to @media screen as well.

I’ve tried the following, but that doesn’t seem to register as the same shared scope for Sass:

@media print { @import "active_admin_datetimepicker"; }

So I’m guessing we should override the variables, or use new ones. We can also override the variables using the !default; flag, which ensures that it doesn’t overwrite them if they ever already exist.

@sunny
Copy link
Contributor
sunny commented Feb 15, 2022

Here are the changes we can apply: https://github.com/sunny/active_admin_datetimepicker/pull/1/files

@Fivell
Copy link
Member
Fivell commented Feb 16, 2022

@workgena @Fivell what about moving away from travis to github actions? cause now specs are not running

@gigorok you're admin for this project, feel free to move forward

@Fivell
Copy link
Member
Fivell commented Feb 17, 2022

@gigorok your PR for CI merged!

@gigorok gigorok force-pushed the gigorok-patch-1 branch 3 times, most recently from 1bb1dea to 16bf8f3 Compare February 17, 2022 11:57
@gigorok gigorok mentioned this pull request Mar 8, 2022
Merged
@gigorok gigorok changed the title activeadmin v2.10.0 supporting activeadmin v2.10.x compatibility Mar 11, 2022
@gigorok
Copy link
Collaborator Author
gigorok commented Apr 7, 2022

active_admin_datetimepicker has been already compatible with activeadmin >= v2.11.0 after changes activeadmin/activeadmin#7341. don't think that we have to support compatibility with v2.10.x

@gigorok gigorok closed this Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gigorok @sunny @Fivell
0