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#3005 : Remove EDI History jquery ui #3416

Merged
merged 5 commits into from
May 1, 2020
Merged

Fix#3005 : Remove EDI History jquery ui #3416

merged 5 commits into from
May 1, 2020

Conversation

stu01509
Copy link
Member

Fixes #3005

Short description of what this resolves:

Remove EDI History jquery ui

image

image

image

image

image

@stu01509
Copy link
Member Author

Hello @bradymiller @stephenwaite @tywrenn

Do you have any ideas or suggestions about these sections? Especially the table seems like used jquery data tables-UI.

image

image

image

@tywrenn
Copy link
Contributor

tywrenn commented Apr 24, 2020

@stu01509 I attempted to fix this up in my pull request that is now basically dead, #2927. If you use some of what I used in cleaning it up that might help you out.

@stu01509 stu01509 changed the title [WIP]Fix#3005 : Remove EDI History jquery ui Fix#3005 : Remove EDI History jquery ui Apr 25, 2020
@stu01509
Copy link
Member Author

Hello @bradymiller @stephenwaite @tywrenn

Finally, It looks better than jquery UI pretty. I also remove datatales-jq to datatables-bs,
Please review it, Thanks :)

image

image

image

image

image

<meta http-equiv="content-type" content="text/html;charset=utf-8" />

<!-- TODO: Address no_bootstrap here !-->
<?php Header::setupHeader(['no_main-theme', 'no_bootstrap', 'datetime-picker', 'datatables', 'datatables-jqui', 'datatables-jqui-theme', 'datatables-scroller', 'datatables-scroller-jqui-theme', 'jquery-ui', 'jquery-ui-sunny']); ?>
<?php Header::setupHeader(['no_main-theme', 'datetime-picker', 'datatables', 'datatables-bs', 'datatables-scroller']); ?>
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

rec dropping the no_main-theme and seeing what happens

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

prob also need this token in there datatables-dt, which is generally needed whenever using datatables-bs

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @bradymiller sir

I removed the no_main-theme, and it seems like to apply your custom style. e.g. button color.
Any suggestions or feedback?

image

@bradymiller
Copy link
Sponsor Member

bradymiller commented Apr 26, 2020

hi @stu01509 , Overall gui appears to look good.

Noted following error in console when opened tab:
image

Also noted following error in console at Notes->Open (enter note) -> Save
image

After get those errors sorted out, I think @stephenwaite may be able to test it out.

@stu01509
Copy link
Member Author

Hi @bradymiller sir

Thanks for your review, I updated the commit, please review it again 😃.

@bradymiller
Copy link
Sponsor Member

bradymiller commented Apr 27, 2020

hi @stu01509 , Looks like getting close. Noted this js error at EDI File->Submit:

image

@stu01509
Copy link
Member Author

Hello @bradymiller

Updated the commit :)

@stephenwaite
Copy link
Sponsor Member

hi @stu01509 , looks like there's a little issue with the edi file
Screenshot from 2020-04-27 09-03-49

@stu01509
Copy link
Member Author

Hello @stephenwaite

Thanks for your review, can you give the test file?
I will try to do completely testing :)

@stephenwaite
Copy link
Sponsor Member

sure, it's attached, thanks @stu01509
EDI_837_5010_Usecase1.txt

@stu01509
Copy link
Member Author

Hello @stephenwaite

I reproduce in my environment, it seems good to work. I guess you deploy old commit version, because I already remove the all jquery-ui accordion. Please take look :)

image

@stephenwaite
Copy link
Sponsor Member

hi @stu01509 , what about round line 800 of interface/billing/edih_view.php?

@tywrenn
Copy link
Contributor

tywrenn commented Apr 28, 2020

@stu01509 Offending code:
$('#x12rsp > #accordion').accordion({

Line 800 to 806 in edih_view.php

@stu01509
Copy link
Member Author

Hello @stephenwaite and @tywrenn

Thanks for your review and help, I updated the commit, please take look.
Thank you :)

@stu01509
Copy link
Member Author

🏓 @stephenwaite

@stephenwaite
Copy link
Sponsor Member

hi @stu01509, am seeing

* e303d8786 - (HEAD, stu01509/fix-#3005-remove-jquery-ui) fix: remove unnecessary code (3 days ago) <Cliff Su>
* a4e3d4b15 - (fix-#3005-remove-jquery-ui) fix: jqeury syantax error (4 days ago) <Cliff Su>
* 0026ba715 - fix: remove no_main-theme and add datatables-dt (4 days ago) <Cliff Su>

as last 3 commits, did you push the update from 2 days ago? thank you

@stu01509
Copy link
Member Author

Hi @stephenwaite

Yes, I fixed the accordion issue in e303d87 commit.

@stephenwaite
Copy link
Sponsor Member

ok, testing well, thank you @stu01509

@stephenwaite stephenwaite merged commit bf85f54 into openemr:master May 1, 2020
@stu01509
Copy link
Member Author

stu01509 commented May 1, 2020

Thanks for your review and help 😁 @stephenwaite

@stephenwaite
Copy link
Sponsor Member

another openemr improvement from @stu01509 !
external-content duckduckgo com

@sjpadgett
Copy link
Member

Aww come on! now I have to go out and find me an animation!:)

@stephenwaite
Copy link
Sponsor Member

stephenwaite commented May 1, 2020

how about
external-content duckduckgo com

@sjpadgett
Copy link
Member

A little ho-hum for my big personality don't ya think?

@stephenwaite
Copy link
Sponsor Member

stephenwaite commented May 1, 2020

external-content duckduckgo com

hope you're not a 🍀 fan

@im-Amitto
Copy link
Member

@stu01509

@stu01509
Copy link
Member Author

stu01509 commented May 2, 2020

Let us say goodbye to outdated animation haha 😄😄

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.

Remove jquery-ui from the Fees->'EDI History' module
6 participants