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

Improve breakpoints #362

Open
7 tasks done
oliverfoster opened this issue Apr 26, 2023 · 16 comments · Fixed by #363 or adaptlearning/adapt_framework#3408
Open
7 tasks done

Improve breakpoints #362

oliverfoster opened this issue Apr 26, 2023 · 16 comments · Fixed by #363 or adaptlearning/adapt_framework#3408
Assignees
Labels
enhancement New feature or request question Further information is requested released

Comments

@oliverfoster
Copy link
Member

oliverfoster commented Apr 26, 2023

Subject of the issue

Adapt breakpoints are slipping further away from devices as they are in the market currently. Below is a rough plan of how to improve the situation whilst retaining backward compatibility.

Step one - Actionable

Step two - Discovery

  • Check whether AAT defaults are derived from the schema in the setup script
    • Schema defaults, need updating, [current ones are incorrect]
  • Allow more than two components in a block
  • Expand vanilla background image config to mirror finalised default breakpoint

Step three - Actionable phase 2

TBD

@oliverfoster oliverfoster added enhancement New feature or request question Further information is requested labels Apr 26, 2023
@oliverfoster
Copy link
Member Author

Thoughts please @eleanor-heath @kirsty-hames @StuartNicholls @guywillis

@kirsty-hames
Copy link
Contributor

kirsty-hames commented Apr 27, 2023

  • Allow more than two components in a block

This is already supported in the FW as there are no limitations assigning components to blocks and using the column classes overrides the _layout. This is not supported in the AAT however as the Page structure UI is limited to the _layout left | right | full display only. I'll add this to the AAT issues.

@kirsty-hames
Copy link
Contributor

Subject of the issue

Adapt breakpoints are slipping further away from devices as they are in the market currently. Below is a rough plan of how to improve the situation whilst retaining backward compatibility.

We could do with padding out what the issues are/what we are trying to achieve as a starting point.

Regarding updating the existing breakpoints, there are many opinions on these so I think it would be good to experiment with different breakpoint values in a real course/s and identify where additional breakpoints are needed.

@oliverfoster
Copy link
Member Author

oliverfoster commented Apr 27, 2023

  • Allow more than two components in a block

This is already supported in the FW as there are no limitations assigning components to blocks and using the column classes overrides the _layout. This is not supported in the AAT however as the Page structure UI is limited to the _layout left | right | full display only. I'll add this to the AAT issues.

Yes, to complicate this issue, there is the question of how wrapping works with varying numbers of components. 3 is a particularly good example for explaining the complexity, with rows (3), (1,2), (2,1), (1,1,1) - how do we decide between a (1,2) and a (2,1) layout in the AAT config?

@oliverfoster
Copy link
Member Author

Regarding updating the existing breakpoints, there are many opinions on these so I think it would be good to experiment with different breakpoint values in a real course/s and identify where additional breakpoints are needed.

In my conversations with people, at least a couple more breakpoints are required, though their actual values seem in contention and so I was going to introduce them unvalued so that we can explore that question further with a testing base.

@oliverfoster
Copy link
Member Author

We could do with padding out what the issues are/what we are trying to achieve as a starting point.

I would very much like to avoid long term goals and start with known issues. There are many intersecting pieces to this puzzle. We know, for example, that we need to move the breakpoints to cater for the modern device stack, and that adding a couple more would help. I'd like to get that done first. That is why I've broken it down into immediately actionable pieces and after that, a further discovery and alignment phase.

@kirsty-hames
Copy link
Contributor

  • Allow more than two components in a block

This is already supported in the FW as there are no limitations assigning components to blocks and using the column classes overrides the _layout. This is not supported in the AAT however as the Page structure UI is limited to the _layout left | right | full display only. I'll add this to the AAT issues.

Yes, to complicate this issue, there is the question of how wrapping works with varying numbers of components. 3 is a particularly good example for explaining the complexity, with rows (3), (1,2), (2,1), (1,1,1) - how do we decide between a (1,2) and a (2,1) layout in the AAT config?

The current implementation of columns uses classes and these handle the wrapping responsively. The application of classes itself isn't very AAT friendly and we do have the added complexity of responsive wrapping so multiple classes needed. Ideally this would be configurable rather than a class and we streamline this for ease. I haven't raised this as an issue on AAT as this needs some consideration in the FW first.

@StuartNicholls
Copy link
Contributor

StuartNicholls commented Apr 28, 2023

We could do with padding out what the issues are/what we are trying to achieve as a starting point.

I would very much like to avoid long term goals and start with known issues. There are many intersecting pieces to this puzzle. We know, for example, that we need to move the breakpoints to cater for the modern device stack, and that adding a couple more would help. I'd like to get that done first. That is why I've broken it down into immediately actionable pieces and after that, a further discovery and alignment phase.

I thought that the main thrust of this change was long term goals. From a production standpoint 'Known issues' / immediate issues and device issues are pretty minor issues in my opinion, so for me it makes it quite difficult to contribute in this context if we're not included long term / complex goals.

We certainly need an extra breakpoint at the desktop end, but I have no idea if we'd need a smaller one at this stage. I guess, from a framework point-of-view some people might say.. do more e-learning on mobile devices?.. I'm just guessing here though. In my experience though, I never need anything below the smaller breakpoint. My initial thoughts are we may only need 4 breakpoints or more at the upper end, especially if we want to keep the middle ones similar to their historical values, but I think this stuff needs some investigation.

@StuartNicholls
Copy link
Contributor

Regarding updating the existing breakpoints, there are many opinions on these so I think it would be good to experiment with different breakpoint values in a real course/s and identify where additional breakpoints are needed.

In my conversations with people, at least a couple more breakpoints are required, though their actual values seem in contention and so I was going to introduce them unvalued so that we can explore that question further with a testing base.

Re. actual breakpoints, I have no issues with specific values, but I'd like to see them justified from either a production standpoint or if its a framework standpoint, then thats fine by me, just wanting some transparency and articulation on the matter.

@StuartNicholls
Copy link
Contributor

  • Allow more than two components in a block

This is already supported in the FW as there are no limitations assigning components to blocks and using the column classes overrides the _layout. This is not supported in the AAT however as the Page structure UI is limited to the _layout left | right | full display only. I'll add this to the AAT issues.

Yes, to complicate this issue, there is the question of how wrapping works with varying numbers of components. 3 is a particularly good example for explaining the complexity, with rows (3), (1,2), (2,1), (1,1,1) - how do we decide between a (1,2) and a (2,1) layout in the AAT config?

If Im following this correctly.. This is going to be a tough one to implement in the tool, but from a framework perspective, I'd imagine layouts at breakpoints will be up to the FED. I think auto reflows / wrapping of components will be very simple as a default?

@oliverfoster
Copy link
Member Author

oliverfoster commented Apr 28, 2023

We could do with padding out what the issues are/what we are trying to achieve as a starting point.

I would very much like to avoid long term goals and start with known issues. There are many intersecting pieces to this puzzle. We know, for example, that we need to move the breakpoints to cater for the modern device stack, and that adding a couple more would help. I'd like to get that done first. That is why I've broken it down into immediately actionable pieces and after that, a further discovery and alignment phase.

I thought that the main thrust of this change was long term goals. From a production standpoint 'Known issues' / immediate issues and device issues are pretty minor issues in my opinion, so for me it makes it quite difficult to contribute in this context if we're not included long term / complex goals.

We certainly need an extra breakpoint at the desktop end, but I have no idea if we'd need a smaller one at this stage. I guess, from a framework point-of-view some people might say.. do more e-learning on mobile devices?.. I'm just guessing here though. In my experience though, I never need anything below the smaller breakpoint. My initial thoughts are we may only need 4 breakpoints or more at the upper end, especially if we want to keep the middle ones similar to their historical values, but I think this stuff needs some investigation.

As in, if we make the breakpoints absolutely derivable from the config, names and values, then it's much easier to figure out which ones we need and the consequences of any change, as there will be a working implementation through which to explore any new values. The breakpoint stuff is not a long term goal in the sense that we know we need it and we know roughly what we don't know and fashioning a pr to explore is very straightforward.

Please add longer term goals, for future discussion, to stage 2 of the issue description as a short summary.

If you could be the first to suggest breakpoint values and a rationale that would kick off that part? It may be wise to open a separate issue for these sub topics and add a link to the separate issue, in the description, on this issue. #364

@StuartNicholls
Copy link
Contributor

StuartNicholls commented Apr 28, 2023

@oliverfoster Okay, we had planned to do some exploitative stuff by hacking in to a framework instance, but if we're prepared to go this route then that makes perfect sense to me.

@oliverfoster
Copy link
Member Author

oliverfoster commented Apr 28, 2023

@oliverfoster Okay, we had planned to do some exploitative stuff though by hacking in to a framework instance, but if we're prepared to go this route then that makes perfect sense to me.

Based upon the two prs attached, whatever one defines in the config.json:screenSize object will be passed through the Less compiler, into less variables @adapt-device-[name], be output as css variables --adapt-device-[name], be available on the config model at Adapt.config.get('screenSize') and be applied as classes size-[name].

I have added two extra attributes to the schemas xsmall and xlarge as an example, but with effectively out-of-bounds values, which now come through with the others, in all of the above regards, but are effectively useless with the values 0 and 2147483647 respectively.

Using the prs, it is possible to create ones own config.json:screenSize object with ones own name/value pairs and the fw/core will carry on. If, however, one were to remove small, medium or large, I expect there to be larger consequences, specifically with referenced less or class styling, than if simply altering their values. For example, if we decide they can be shifted upwards and a smaller value needs introducing at the bottom, that's fine. If we decide to need two larger values, then we can do that. I would suggest, from a practical perspective, to leave the existing names untouched for both backward compatibility and to reduce the rework effort. The solution is completely name/value agnostic, except for the schema, hardcoded defaults, the existing Less and the wider ecosystem into which any solution must fit. The hardcoded defaults only apply if no config.json:screenSize object is defined in the compiled source, which usually would mean, that no schema defaults are defined also. I may remove the hardcoded defaults altogether, pending review, and rely solely on the schema to provide defaults.
Refs: fw defaults, core defaults, schema defaults (defaults come from the legacy schema)

This change should allow explorative stuff, without hacking and without changing the existing behaviour, just change the config.json:screenSize and schema defaults accordingly.

@StuartNicholls
Copy link
Contributor

There is some value in having the breakpoints abbreviated somewhat, so I'd imagined something along the lines of xs, sm, md, lg, xl as that will help greatly with the columns fix (I'll explain that in step two). But more than 100% agree we shouldn't remove small / medium / large as that will have a big impact as you say. I had thought about ways we could do this (move forward and retain legacy names), but might it be easier to chat that bit though after I've added some rationale up top there and thought it though a bit more. If not possible, then fine, it just makes other bits a little more cumbersome.

Size-wise. My thinking was just to tweak the middle values so things would still work if an old theme was dropped into a new set of values (assuming only adapt breakpoints were used of course). But the tweaks would target devices better and make the page wider to reflect wider screens on desktop. So 760 becomes say 768 for example (or 800 depending on whats important device-wise).

@oliverfoster
Copy link
Member Author

oliverfoster commented May 4, 2023

In the prs, I have removed new xsmall and xlarge and removed old hardcoded defaults, such that screen sizes must be defined in the config.json or they will be derived from the schema defaults.

#363
adaptlearning/adapt_framework#3408

These prs can be approved and merged, so that we can more easily test any changes to breakpoint sizes.

@oliverfoster
Copy link
Member Author

🎉 This issue has been resolved in version 6.35.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment