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

MF-828: The date field in the start visit form is truncated at small display widths #415

Closed
wants to merge 0 commits into from

Conversation

jnsereko
Copy link
Contributor

@jnsereko jnsereko commented Oct 3, 2021

…display widths

Requirements

  • This PR has a title that briefly describes the work done, including the ticket number if there is a ticket.
  • My work conforms to the OpenMRS 3.0 Styleguide.
  • I checked for feature overlap with existing widgets.

Summary

Reduced the Text Date size to allow wraping

Screenshots

7732bd70-c9f2-4f0e-b8c1-0fc894427d51.mp4

None.

Issue

https://issues.openmrs.org/browse/MF-828

Other

None.

Sorry, something went wrong.

@brandones brandones requested a review from donaldkibet October 5, 2021 15:45
@jnsereko
Copy link
Contributor Author

jnsereko commented Oct 7, 2021

Maximized view
image

Minimized View
image

cc @donaldkibet @brandones @samuelmale

@brandones
Copy link
Contributor

Sorry this got dropped @jnsereko . @donaldkibet and @jnsereko are you able to help finish this?

@jnsereko
Copy link
Contributor Author

hello @brandones @donaldkibet
I am sorry for taking long on this. I lost its truck. I have added changes. I kindly request for a review

@github-actions
Copy link
Contributor

github-actions bot commented Feb 22, 2022


Error: Error while trying to collect info after merging MF-828 into master.

Error: Command failed: git fetch --no-tags --prune origin refs/pull/415/merge
fatal: couldn't find remote ref refs/pull/415/merge

    at ChildProcess.exithandler (node:child_process:397:12)
    at ChildProcess.emit (node:events:394:28)
    at maybeClose (node:internal/child_process:1067:16)
    at Socket. (node:internal/child_process:453:11)
    at Socket.emit (node:events:394:28)
    at Pipe. (node:net:672:12)

Generated by @jsenv/file-size-impact during Report bundle size#1959782438 on 1ca6ebb

@jnsereko
Copy link
Contributor Author

jnsereko commented Feb 22, 2022

Below is a visual expression of the changes

add.visit.form.mp4

Comment on lines 86 to 88
:global(.bx--time-picker), :global(.bx--date-picker-container), :global(.bx--date-picker), :global(#visitStartDateInput) {
width: 100%;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

:global styles on Carbon classes should go in esm-styleguide.

The global style on #visitStartDateInput should be changed to a normal scoped class style, i.e.

.visitStartDateInput {
  width: 100%;
}

@brandones
Copy link
Contributor

@jnsereko I'm sorry this keeps getting stale. I'll be reviewing it from here forward.

Your implementation does not fit the designs: https://app.zeplin.io/project/60d59321e8100b0324762e05/screen/61bb8ae18b41ad4d775c2c0c

Screenshot from 2022-03-04 19-19-40

@jnsereko
Copy link
Contributor Author

jnsereko commented Mar 6, 2022

Hello @brandones
I had only focused on the review and advice from @donaldkibet and forgot to check the design.
I have pushed some changes but if they don't match the requirements, I am willing to unassign.
image

image

Thank you for the support.

@brandones
Copy link
Contributor

@jnsereko Thanks. Please make the date and time inputs flow onto separate lines when the viewport is small.

@jnsereko
Copy link
Contributor Author

jnsereko commented Mar 9, 2022

hello @brandones @donaldkibet
In my thinking, adding row-column is my best alternative
image

@brandones
Copy link
Contributor

That looks good. What does it look like at wider display widths?

@jnsereko
Copy link
Contributor Author

jnsereko commented Mar 9, 2022

hey @brandones it goes into the same line at widith: 476 and its in different lines below this widith
image

@jnsereko jnsereko closed this Mar 9, 2022
@jnsereko
Copy link
Contributor Author

jnsereko commented Mar 9, 2022

hello @brandones
I accidentally closed this while rebasing. I was a little sleepy. Please kindly lets continue with this from #600

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.

None yet

4 participants