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.

Sign up for GitHub

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

Jump to bottom

New: Added xlarge breakpoint, added isScreenSizeMin #372

Merged
merged 11 commits into from
May 12, 2023
Merged

New: Added xlarge breakpoint, added isScreenSizeMin #372

merged 11 commits into from
May 12, 2023

Conversation

oliverfoster
Copy link
Member

@oliverfoster oliverfoster commented May 5, 2023 •
edited
Loading

fixes #371

Alternative to #373

New

  • Add xlarge default to schemas starting at 1280
  • Add device.isScreenSizeMin(name) to return true/false if the device is equal or above the named screen size

Fix

  • Adjusted the breakpoints and schema descriptions so that it's clear that the breakpoint values are for the bottom of each named boundary according to Improve breakpoints: Realign existing breakpoints to reflect modern changes in device sizes #364 (comment)
  • Recalculated size- classes were not derivable
  • mobileInstruction now only appears at small, to account for the small > medium change in hotgraphic and narrative
  • When only small and large images are defined, images switch from small to large at medium or above to better fit the change from 1 component to 2 component layouts

Breakpoints

All breakpoints are multiples of 8.

Name Value Range Distance Components Columns Deviceish
small 0 0- 719 720 1 4 mobile portrait
medium 720 720 - 959 240 2 8 portait tablet, landscape mobile
large 960 960 - 1279 480 2 12 large portrait tablet, landscape tablet, small desktop, laptop, netbook
xlarge 1280 1280+ n/a 2 12 desktop, laptop, etc
  • small is for single component views
  • medium and above are for double component views
  • xlarge is at 1280 to match up with Google M1 breakpoints
  • @max-page-width at 1440, for limiting the maximum container widths
  • Edge cases for ultra small devices can be specifically created in a theme

Rationale from #364

Testing

Use:

@oliverfoster oliverfoster added the enhancement New feature or request label May 5, 2023
@oliverfoster oliverfoster self-assigned this May 5, 2023
@oliverfoster oliverfoster mentioned this pull request May 5, 2023
Closed
@oliverfoster oliverfoster changed the title New: Added xsmall breakpoint New: Added xsmall breakpoint, add isScreenSizeFrom May 5, 2023
@oliverfoster oliverfoster added bug Something isn't working high priority labels May 5, 2023
@oliverfoster oliverfoster mentioned this pull request May 5, 2023
Closed
@joe-replin
Copy link
Contributor

#373 (comment)

@oliverfoster oliverfoster changed the title New: Added xsmall breakpoint, add isScreenSizeFrom New: Added xsmall breakpoint, add isScreenSizeMin May 10, 2023
@oliverfoster oliverfoster changed the title New: Added xsmall breakpoint, add isScreenSizeMin New: Added xsmall breakpoint, add isScreenSizeMin/Max May 10, 2023
This was referenced May 10, 2023
Closed
Closed
Closed
Merged
Merged
Merged
guywillis
guywillis approved these changes May 11, 2023
View reviewed changes
Copy link
Contributor

@guywillis guywillis left a comment

Choose a reason for hiding this comment

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

👍

kirsty-hames
kirsty-hames reviewed May 11, 2023
View reviewed changes
Copy link
Contributor

@kirsty-hames kirsty-hames left a comment

Choose a reason for hiding this comment

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

The consensus on the additional breakpoint has since changed (as per PR #364 (comment)) but I can confirm the mobileInstruction and image small and large work as expected.

@oliverfoster
Copy link
Member Author

@kirsty-hames have updated the pr accordingly

@oliverfoster oliverfoster changed the title New: Added xsmall breakpoint, add isScreenSizeMin/Max New: Added xlarge breakpoint, added isScreenSizeMin/Max May 11, 2023
cahirodoherty-learningpool
cahirodoherty-learningpool approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

swashbuck
swashbuck requested changes May 12, 2023
View reviewed changes
js/device.js Outdated Show resolved Hide resolved
js/device.js Show resolved Hide resolved
templates/header.jsx Outdated Show resolved Hide resolved
@oliverfoster oliverfoster changed the title New: Added xlarge breakpoint, added isScreenSizeMin/Max New: Added xlarge breakpoint, added isScreenSizeMin May 12, 2023
swashbuck
swashbuck approved these changes May 12, 2023
View reviewed changes
Copy link
Contributor

@swashbuck swashbuck left a comment

Choose a reason for hiding this comment

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

👍

@oliverfoster oliverfoster merged commit 16b2738 into master May 12, 2023
@oliverfoster oliverfoster deleted the issue/371 branch May 12, 2023 15:22
github-actions bot pushed a commit that referenced this pull request May 12, 2023
@semantic-release-bot
Chore(release): 6.36.0
c54cffe 
# [6.36.0](v6.35.0...v6.36.0) (2023-05-12)

### New

* Added xlarge breakpoint, added isScreenSizeMin (#372) ([16b2738](16b2738)), closes [#372](#372)
@oliverfoster
Copy link
Member Author

🎉 This PR is included in version 6.36.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@oliverfoster oliverfoster mentioned this pull request May 12, 2023
Closed
This was referenced May 15, 2023
Closed
Closed
kirsty-hames added a commit to cgkineo/adapt-hotgrid that referenced this pull request May 15, 2023
@kirsty-hames
FW version bumped inline with core code
e1094fc 
requires adaptlearning/adapt-contrib-core#372
@kirsty-hames kirsty-hames mentioned this pull request May 15, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@swashbuck swashbuck swashbuck approved these changes

@guywillis guywillis guywillis approved these changes

@cahirodoherty-learningpool cahirodoherty-learningpool cahirodoherty-learningpool approved these changes

@kirsty-hames kirsty-hames Awaiting requested review from kirsty-hames

@StuartNicholls StuartNicholls Awaiting requested review from StuartNicholls

@eleanor-heath eleanor-heath Awaiting requested review from eleanor-heath

@cksachdev cksachdev Awaiting requested review from cksachdev

Assignees

@oliverfoster oliverfoster

Labels
bug Something isn't working enhancement New feature or request high priority released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve Breakpoints - Mismatch between js and css
6 participants
@oliverfoster @joe-replin @swashbuck @kirsty-hames @guywillis @cahirodoherty-learningpool