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

🪲 fix custom adventures' alignment #5297

Merged
merged 13 commits into from
Mar 26, 2024
Merged

🪲 fix custom adventures' alignment #5297

merged 13 commits into from
Mar 26, 2024

Conversation

hasan-sh
Copy link
Collaborator

@hasan-sh hasan-sh commented Mar 21, 2024

Fixes #5296
Fixes #5293

Description
In this PR, I attempted to split the teachers' adventures into two columns optimally. This is done by splitting the tags rather than the text, as we currently do. Furthermore, I've set a maximum height on the code blocks to equal the editor's height. This is to avoid very long code blocks which would most likely smaller than other information within the adventure content.

How to test?

  • create long adventures
  • add them to your class
  • preview the class and whether the adventure is split intow 2 columns, the best they could!

ALSO this PR should fix how currently live adventures, please try to find public adventures and test them locally. Here's some:

@hasan-sh hasan-sh marked this pull request as ready for review March 25, 2024 11:38
Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

I just tested it and it works well for new adventures, but not entirely correct yet for existing adventures (I forst edited an existing one)

image

Can you take a look at that?

Copy link
Member

@Felienne Felienne left a comment

Choose a reason for hiding this comment

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

Ah sorry I meant request changes not approve!

@hasan-sh
Copy link
Collaborator Author

I just tested it and it works well for new adventures, but not entirely correct yet for existing adventures (I forst edited an existing one)

image Can you take a look at that?

I believe it has to do with formatting adventure's code! Could you tell me what you did to reproduce this?

@Felienne
Copy link
Member

I just tested it and it works well for new adventures, but not entirely correct yet for existing adventures (I forst edited an existing one)
image
Can you take a look at that?

I believe it has to do with formatting adventure's code! Could you tell me what you did to reproduce this?

I selected one of my existing adventures (created before the automatic curly addition, so maybe that is related?), and added more lines of code to the code example with copy paste.

@hasan-sh
Copy link
Collaborator Author

@Felienne it was from that change indeed as was another one that i fixed here as well. I'll keep on any possible errors of that PR.

This should be fixed now!

@Felienne
Copy link
Member

@Felienne it was from that change indeed as was another one that i fixed here as well. I'll keep on any possible errors of that PR.

This should be fixed now!

Yes, the newlines are fixed indeed:

image

Although I still get the curlies. This is not an issue for this PR, but I will make a new one for @jpelay

@hasan-sh
Copy link
Collaborator Author

hasan-sh commented Mar 25, 2024

@Felienne it was from that change indeed as was another one that i fixed here as well. I'll keep on any possible errors of that PR.
This should be fixed now!

Yes, the newlines are fixed indeed:

Although I still get the curlies. This is not an issue for this PR, but I will make a new one for @jpelay

@jpelay the problem is that the formatted_content is only converting
tags in the front-end AND when you save the adventure, therefore existing adventrues have this problem. Here i simply passed the content which may have curly braces for existing adventures 09d26c3. I'd do that step in the backend instead!

@Felienne
Copy link
Member

@Felienne it was from that change indeed as was another one that i fixed here as well. I'll keep on any possible errors of that PR.
This should be fixed now!

Yes, the newlines are fixed indeed:
Although I still get the curlies. This is not an issue for this PR, but I will make a new one for @jpelay

@jpelay the problem is that the formatted_content is only converting tags in the front-end AND when you save the adventure, therefore existing adventrues have this problem. Here i simply passed the content which may have curly braces for existing adventures 09d26c3. I'd do that step in the backend instead!

Thanks for the explanation and for the suggested fix. Will you do that in this PR?

@hasan-sh
Copy link
Collaborator Author

hasan-sh commented Mar 26, 2024

Will you do that in this PR?

I think we should discuss it first with @jpelay, i think i'd make more sense to take the code related to adding curly braces automatically to the backend. I'm just wondering if there's a reason why this is in the frontend.

Copy link
Contributor

mergify bot commented Mar 26, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 5fd1fb6 into main Mar 26, 2024
12 checks passed
@mergify mergify bot deleted the fix-custom-adv-columns branch March 26, 2024 13:39
Copy link
Contributor

mergify bot commented Mar 26, 2024

Thank you for contributing! Your pull request is now going on the merge train (choo choo! Do not click update from main anymore, and be sure to allow changes to be pushed to your fork).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🪲 custom adventure in two columns 🪲 all <br>'s should be replaced, not the first one only!
2 participants