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

Decide approach for style sheets - migrate styling #19

Closed
tomzemp opened this issue Jun 3, 2019 · 7 comments
Closed

Decide approach for style sheets - migrate styling #19

tomzemp opened this issue Jun 3, 2019 · 7 comments
Assignees
Labels
MVP Minimum Viable Product (first React Rollout)

Comments

@tomzemp
Copy link
Contributor

tomzemp commented Jun 3, 2019

Decide approach (e.g. scss, stylable) for handling styling and remove existing-in line syling out of components

@tomzemp tomzemp self-assigned this Jun 3, 2019
@tomzemp tomzemp added the MVP Minimum Viable Product (first React Rollout) label Jun 3, 2019
tomzemp pushed a commit that referenced this issue Jun 21, 2019
tomzemp pushed a commit that referenced this issue Jun 24, 2019
@tomzemp
Copy link
Contributor Author

tomzemp commented Jul 3, 2019

Classes are being rendered differently from within dhis2 (dev-tom.datim)

image

Then when running locally:
image

This is causing the display to vary wildly when on dhis2 server (see e.g. #27 (comment))

Need to investigate/fix. Maybe adding MUI wrapper will be sufficient, or maybe need to move away from styling with @material-ui/styles hooks (https://material-ui.com/styles/basics/)

@tomzemp
Copy link
Contributor Author

tomzemp commented Aug 9, 2019

@benguaraldi @jakub-bao: I redid the styling by using vanilla in-line styling on the components rather than trying to use Material-UI's makeStyles...it seems to achieve the same thing, and be roughly similar in terms of where the styling lives, but it doesn't result in problems when loaded in an actual DHIS2 instance as far as I can see.

I've put this on redoStyles branch in case you want to weigh in on this alternative approach

@benguaraldi
Copy link
Contributor

@tomzemp Much better! Thank you for tracking that down! Sorry that it was super-annoying.

I don't know if these are easy fixes or if you want additional tickets for them, but two things:

  • If the IM and Legacy Partner Name could default to twice the width of the other columns, that would be great
  • We have to scroll down to the bottom of the table to scroll across the table—I feel like there was a fix for this in the previous version, where the page was the width of the table? Let me know if you want more clarification on this

I'm fine with postponing those both to after the MVP

@tomzemp
Copy link
Contributor Author

tomzemp commented Aug 27, 2019

@benguaraldi: updated the column widths
image

Changing the scroll is unfortunately difficult so far as I can tell :-(

@benguaraldi
Copy link
Contributor

@tomzemp Column widths look great! I'll make another ticket about the scroll bar. It does look difficult.

I'm noticing that the table and several other divs have min-width: 3.40282e+38px;, which leads to this:
Screen Shot 2019-08-28 at 3 24 07 AM

Note that the scroll bar is almost completely on the left—there's thousands of pixels of whitespace to the right. Are you seeing that? Is it easy to fix?

tomzemp pushed a commit that referenced this issue Aug 29, 2019
@tomzemp
Copy link
Contributor Author

tomzemp commented Aug 29, 2019

Thanks for noticing that @benguaraldi! I guess I was just looking at the first couple of columns.

I've fixed and redeployed on dev-tom.datim
image

@tomzemp
Copy link
Contributor Author

tomzemp commented Aug 29, 2019

This issue has been completed;

scroll bar mentioned in comments is on separate issue:
#16

@tomzemp tomzemp closed this as completed Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MVP Minimum Viable Product (first React Rollout)
Projects
None yet
Development

No branches or pull requests

2 participants