charludo / ycms

Apache License 2.0
0 stars 0 forks source link

Feature/responsive design #127

Closed noworneverev closed 8 months ago

noworneverev commented 8 months ago

Short description

Improve the responsiveness of the website

Proposed changes

~Pending:~ ~- overlap when opening sidebar in patient intake, already know that if removing all position: relative can solve the issue, including tom-select's position: relative (don't know how to remove this one)~

Side effects

Resolved issues

Fixes: #125

charludo commented 8 months ago

Awesome, thank you so much for tackling this issue.

Really, really nice progress already. I noticed a couple of additional things:

overlap when opening sidebar in patient intake, already know that if removing all position: relative can solve the issue, including tom-select's position: relative (don't know how to remove this one)

As far as I can tell, it's enough to set a higher z-index on both the #sidebar and #sidebarBackdrop. They are currently set to z-100, try z-[999] and z-[998], respectively. This also fixes the issue for the timeline!

noworneverev commented 8 months ago

@charludo Not sure why the test fails after rebasing the latest develop branch. It can be solved by adding checks to cached property. Should I modify this?

    @cached_property
    def current_admission_date(self):
        """
        Helper property for accessing the patient's current admission date

        :return: the current admission date
        :rtype: str
        """
        if not self.current_stay:
            return None
        return self._get_date_status(self.current_stay.admission_date)

    @cached_property
    def current_discharge_date(self):
        """
        Helper property for accessing the patient's current discharge date

        :return: the current discharge date
        :rtype: str
        """
        if not self.current_stay:
            return None
        return self._get_date_status(self.current_stay.discharge_date)
charludo commented 8 months ago

@charludo Not sure why the test fails after rebasing the latest develop branch. It can be solved by adding checks to cached property. Should I modify this?

    @cached_property
    def current_admission_date(self):
        """
        Helper property for accessing the patient's current admission date

        :return: the current admission date
        :rtype: str
        """
        if not self.current_stay:
            return None
        return self._get_date_status(self.current_stay.admission_date)

    @cached_property
    def current_discharge_date(self):
        """
        Helper property for accessing the patient's current discharge date

        :return: the current discharge date
        :rtype: str
        """
        if not self.current_stay:
            return None
        return self._get_date_status(self.current_stay.discharge_date)

Hey, sorry for not seeing this earlier. Apologies, this is my fault. I forgot to add a test patient which we could use for tests indefinitely. One of the patients That were used in the tests must have been discharged yesterday evening.

I have added this test case here. It's a single patient which will stay in the hospital for another 5 years... 😄 That should hopefully do it ^^

The first thing _get_date_status() does is check if the passed date is None, so in actual usecases we are already safe. IDK if the additional checks are necessary, since this time, they did uncover an actual error. Then again, they don't hurt either, so... Your choice if you want to keep them in.

noworneverev commented 8 months ago

Nice! Again, thank you so much for doing this issue.

I have a couple small requests:

  • the ward/timeline switch is much better now, except it's not aligned with the "Ward" title above it. That one has a x-padding of 4, maybe we could remove that on small screens?

Do You mean space-x-4 in ward_selection_form.html? If so, I think this one is necessary, otherwise the select element is too close to the word "Ward".

  • when assigning/transfering a patient, could you break up the "More reassignment options" and the drop down? Just setting md:flex or lg:flex instead of just flex does the trick. And then the "Unassign" button below it should also lose its left padding on small screens

ok

  • the success/error notifications still cling to the left of the screen. Setting the top and right values to md:top-8 md:right-8 on #messages in _raw.html does the trick.

But I already set top and right like follows, I did't see any different after I add md:top-8 md:right-8.

            <div id="messages"
                 class="absolute top-8 right-8 flex flex-col max-w-screen-sm z-50">
  • the number of unsaved bed assignments in the ward view is still unreadable on mobile. Setting flex-wrap on #change-counter and wrapping the span and the "unsaved bed assignment changes" in a div works, I think. Alternatively, if you find a way to also make the 2 buttons take the full width together, that would look ieven nicer, but not necessary

ok

  • the header of the patient list can probably just be hidden on mobile

I think it's needed, no reason to show different context in different screen size, except the layout.

  • I get what you did with the table, but it's really necessary that we are still able to access the details from there 😅 I get that this table is a fair bit of work though, if you think it's out of scope for this PR, feel free to instead create a new issue.

It's tricky to make the table responsive. This is the best I can do for now, but I just noticed that on a small screen, the fields, except for the name field, can not be edited since I intentionally hid them.

charludo commented 8 months ago

Do You mean space-x-4 in ward_selection_form.html? If so, I think this one is necessary, otherwise the select element is too close to the word "Ward".

No, in ward_selection_form, replace p-4 with md:px-4 py-4 (right next to space-x-4).

Without: image With: image

charludo commented 8 months ago

It's tricky to make the table responsive. This is the best I can do for now, but I just noticed that on a small screen, the fields, except for the name field, can not be edited since I intentionally hid them.

Alright, then please open a new issue for that.

charludo commented 8 months ago

Nice! Please squash the commits, and then it's ready to merge.