Thanks @mksd for unmasking this bug.
The issue was: Some of the concept uuids may contain leading or trailing spaces(maybe the parsing didn’t mind that) hence couldn’t not be retrieved from the DB giving us null concept values in the conceptMap . A conceptMap with null values is responsible for having the blank redundant columns.
Opened Ticket at: [RA-1680] - OpenMRS Issues
cc: @mogoodrich
@samuel34 is it possible to check the rest of the dashboard widgets and apply a single fix across all of them?
I am thinking doing this for any concept options higher up to remove any spaces from the attributes
@mozzy unfortunately we did a regression test of the PIH EMR distro last week and have about 12 or so outstanding issues or so that we discovered. We plan to look into them in greater detail this week. I believe that a lot of them are in non-Ref App modules so wouldn’t have been something that would have been easy to discover if you weren’t using the PIH EMR. As we review them, I’ll flag any major ones that apply to the Ref App as a whole here.
One thing we have been thinking of is if there should/could be a way to say “don’t include bootstrap on this page” to support legacy pages that may not have been upgraded to use the Bootstrap changes. We will follow as we look into this further.
I would also recommend anyone else with their own custom distro should probably do a comprehensive test for any conflicts.
fyi @cioan
@ssmusoke @mozzy @samuel34 so we have a couple relatively stand-alone apps that were designed using Bootstrap 3 and so including Bootstrap 4 wrecks their formatting.
We’ve added configuration parameter “includeBootstrap” to standardEmrPage that defaults to true but allows implementations to disable the inclusion of Bootstrap if they so desire. Ticket and PR here, let us know your thoughts, thanks!
great work @mogoodrich
@mogoodrich This looks like a great approach for resolving the Bootstrap integration issue
@ssmusoke @mozzy I found another issue regarding FontAwesome and font-width… the details, a link, and fix documented in the ticket here:
I just tested that this fixed things with the older FontAwesome library, would be good if someone could test that things are still working (and nothing is broken!) with FontAwesome 5.
just checked this page here
https://qa-refapp.openmrs.org/openmrs/uicommons/icons.page ,
and all FontAwesome 5 icons are still working fine
Just added one more PR here to address another issue:
One more change… this one I committed directly to UI Commons without a PR because I wanted to push it out to our CI environment so others could test. However, it could be high-impact change, so worth others looking into.
Ticket is linked below, but the summary is that there seemed to be an excess of padding around the margins after the bootstrap upgrade, and, looking closer, Bootstrap applied some padding, but there was already existing padding applied to the main body-wrapper class. I removed that padding as part of this commit.
fyi, I’m in the process of doing releases of a few of the RefApp modules so we get “stable” post-bootstrap integration versions of these modules. Generally, this shouldn’t cause any issues, but if it is for anyone, please let me know!
@mogoodrich we shall also need to release another minor version of coreapps , because all the fix for ConditionList are contained in the latest snapshot .
Thanks @mozzy… I did a release of coreapps yesterday as well… the release build actually failed, but I believe the actual release worked, let me know if you notice any issues.
That being said, I think we have follow-on PRs and fixes for the condition list that aren’t in yet. But it shouldn’t be a problem to do new another release if necessary. Through CI releases are generally pretty easy and we should get in the habit of doing them more frequently.
Take care, Mark
thats true let try to follow them up,
@mogoodrich did you release version 1.25.0 of core apps ?? i cant see that version updated in the reff-app-distro
the last upgrade am seeing is to 1.24.0 ,which was done several weeks ago
Its fine , i have seen the new tag 1.25.0 on the core apps repo. ill just do the update in the reff-app-distro manualy
We also still have unnecessary effects on the Patient Dashbord ,because of increased margins,
Normal page
Thanks @mozzy! Yes, I released 2.25.0 coreapps. The build failed at the very end, but everything seemed to work… I must have missed that the ref app did not upgrade, sorry about that!
Regarding the margins, I though we had fixed it, or at least gotten it close to want it used to be… below is what we are seeing in our implementation, and I thought everything we did would be applied to the ref app as well, but maybe I missed something. The QA Ref app server had been down last week when I was testing, but it looks like it’s up now… and, yes, I see, it looks like the wrapping is still a problem on the Ref App. I’ll try to look into it later, I think the issue, at least in our implementation, was a container class being nested within a container class.
was is it fixed from Ui Commons ?? Which version of Ui commons do you run on , the reff app runs on 2.10.0
I just released UI Commons on Wednesday, so 2.10.0 is the latest and should have all the bootstrap fixes we’ve made so far.
