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

Modernization #166

Merged
merged 19 commits into from
Nov 28, 2023
Merged

Modernization #166

merged 19 commits into from
Nov 28, 2023

Conversation

glaszig
Copy link
Contributor

@glaszig glaszig commented Sep 2, 2023

i realize this is quite a bucket of stuff but i couldn't help myself.

  • elevates supported ruby and rails versions to 3.0 and 7.1, respectively, as ruby 2 and rails < 6.1 are EOL
  • eliminate database requirement as nothing in this gem is actually needing one
  • convert coffeescript to vannilla js
  • get rid of jquery; no need to bother users with an additional 20k of js
  • position dialog with pure css
  • bumped version number to 4.0.0
  • move off of travis

resolves #121, #132.

@gregschmit
Copy link
Owner

This is awesome; I just want to do a check myself that the CS to JS matches my own output (if you can let me know if there were any additional changes you made that would be helpful) and I'll get this merged ASAP.

@glaszig
Copy link
Contributor Author

glaszig commented Sep 5, 2023

quite a bunch has changed when ripping out jquery: ab263ea...364f156

@glaszig
Copy link
Contributor Author

glaszig commented Sep 5, 2023

don't hurry. please have proper look. i tried to do my best but i'm sure i missed things.

the click triggers before the change event. if that click event gets its default prevented, the change event will not occur.
@fwininger
Copy link

@gregschmit any update ?

mainly adjust to changes in AV::Helpers::Tags::Base

see rails/rails#48574
@glaszig
Copy link
Contributor Author

glaszig commented Nov 11, 2023

i adjusted for rails 7.1. should be ready to be merged.

.github/workflows/test.yml Outdated Show resolved Hide resolved
app/assets/javascripts/recurring_select.js Show resolved Hide resolved
app/assets/javascripts/recurring_select.js Outdated Show resolved Hide resolved
app/assets/javascripts/recurring_select.js Outdated Show resolved Hide resolved
@gregschmit
Copy link
Owner

gregschmit commented Nov 28, 2023

Sorry everybody; a lot of crap in my life has conspired to keep me away from this. I am just going to do some basic testing on a demo app (and I'll likely also test this in active scaffold and active admin) to ensure nothing breaks and then I'll merge this. I'm hoping to get to it within the next day or so.

@glaszig
Copy link
Contributor Author

glaszig commented Nov 28, 2023

fwiw, there’s the dummy app that gives you a bunch of selects fo play with.

@gregschmit
Copy link
Owner

Yeah I remembered that as soon as I hit enter -- I was thinking of another project I'm working on where one of my tasks is to build a demo app for it.

@gregschmit
Copy link
Owner

Ok, gonna merge this now as it looks good to me.

@gregschmit gregschmit merged commit a2ad1d0 into gregschmit:master Nov 28, 2023
@gregschmit
Copy link
Owner

Thanks @glaszig!

@gregschmit
Copy link
Owner

Also thank you @fwininger -- everyone please feel free to @ me on PRs and issues that you believe are important for this project. There are a lot of old PRs so it's hard to separate signal from noise so if you @ me then there's a good chance I'll merge it.

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

Successfully merging this pull request may close these issues.

Day selection stops working when re-selecting weekly option in custom schedule
4 participants