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

[DP-180] Fixed mobile display of lesson pages #229

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chanmioh
Copy link
Collaborator

@chanmioh chanmioh commented Oct 6, 2022

Changelog

Original bug: On mobile, some elements on the WP page cause the page to overflow, so the whole page will look like this:

image

To fix this, I added mobile query styling to these elements. I didn't want to mess with the WP page elements like blockquote's inline styles too much, so instead of using min-width I use max-width to override inline styles on the mobile sizes. Lmk if this should be done another way, but that's how I approached this.

  • wordpress.tsx: Checked for URL path before parsing to see if the created stylesheet needs to be included.
  • lesson.css.ts: Added styles for elements specifically on mobile screen sizes. I also added some styles applied to all screen sizes since I thought they'd look a bit better, I elaborate those below.

WP Changes

Modified WP page: https://wp.dailp.northeastern.edu/wp-admin/post.php?post=673&action=edit

  • Created new class called fill-blank that styles questions where the input box appears within a sentence, and also contains a word below it. I replaced the previous way it was styled into a flexbox for both mobile AND desktop, so that it would be responsive for both sizes.

Before and after on mobile:
image image

  • Created new class called module-buttons that repositions the buttons at the end of the lesson. It was positioned awkwardly on mobile, so I turned it into a flexbox for responsiveness on mobile + desktop.

Before and after on mobile:
image image

@LN7-cyber LN7-cyber self-assigned this Dec 14, 2022
@LN7-cyber
Copy link
Contributor

These changes are working well, Chan Mi. The repositioning of buttons at the end of the page work especially well for mobile users.

website/src/components/lesson.css.ts Show resolved Hide resolved
website/src/components/lesson.css.ts Show resolved Hide resolved
// A global style that styles fill in the blank sentences where a word
// appears below the input box.
globalStyle(`${lesson} .fill-blank span`, {
display: "inline-flex",
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does inline-flex do? Why not normal flex?

website/src/components/lesson.css.ts Show resolved Hide resolved
display: "flex",
flexDirection: "column",
border: `${thickness.thin} solid ${colors.text}`,
...(marginY(vspace.large) as Styles),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised this as clause is necessary, it wasn't passing typecheck? Later on, padding seems to work fine so maybe revise the type of the marginY function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do you mean revise the type that marginY returns? Since it looks like the padding functions that work return a Styles instead of StyleRules which allows them to work

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try this out chanmi, and deploy changes after @GracefulLemming reviews.

website/src/components/lesson.css.ts Show resolved Hide resolved
website/src/components/wordpress.tsx Outdated Show resolved Hide resolved
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.

3 participants