Skip to content

SCENARIO: Simplified Media Insertion (see #365).#394

Open
NourhaneBoudaoud wants to merge 3 commits into
mainfrom
fix-365
Open

SCENARIO: Simplified Media Insertion (see #365).#394
NourhaneBoudaoud wants to merge 3 commits into
mainfrom
fix-365

Conversation

@NourhaneBoudaoud
Copy link
Copy Markdown

@NourhaneBoudaoud NourhaneBoudaoud commented Mar 24, 2026

Co-authored-by: Nourhane BOUDAOUD nourhane.boudaoud@utt.fr
Co-authored-by: Qi ZHENG qi.zheng@utt.fr
Co-authored-by: Zhenpeng LI zhenpeng.li@utt.fr

We, Nourhane BOUDAOUD, Qi ZHENG & Zhenpeng LI hereby grant to Hyperglosae maintainers the right to publish our contribution under the terms of any licenses the Free Software Foundation classifies as Free Software Licenses.

@NourhaneBoudaoud NourhaneBoudaoud self-assigned this Mar 24, 2026
@NourhaneBoudaoud NourhaneBoudaoud added the Good first issue Issue that is easy to test and fix for new team members or external contributors label Mar 24, 2026
@NourhaneBoudaoud NourhaneBoudaoud linked an issue Mar 24, 2026 that may be closed by this pull request
6 tasks
Copy link
Copy Markdown
Member

@benel benel left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request @Hypertopic/tutos.

To improve other scenarios to come, please read commented lines.

Please note also that commit titles should be very strictly named (check Moodle for the rules to apply).

For this particular enhancement, there is a far simpler way to think about this improvement: see what I wrote on the issue discussion (#365). You will see that the scenario (and tests and implementation) will be far easier to update.
You only have to change one line in this scenario.

Comment thread frontend/scenarios/include_video.feature Outdated
Comment thread frontend/scenarios/include_video.feature Outdated
Comment thread frontend/scenarios/include_video.feature Outdated
Comment thread frontend/scenarios/include_video.feature Outdated
Comment thread frontend/scenarios/include_video.feature Outdated
@benel
Copy link
Copy Markdown
Member

benel commented Mar 31, 2026

Please note that your git history will have to be edited so that, for scenarios, you have:

I strongly recommend using Git Fork for the intensive git history rewriting done in IF05.

@NourhaneBoudaoud NourhaneBoudaoud marked this pull request as ready for review April 7, 2026 13:28
@benel
Copy link
Copy Markdown
Member

benel commented Apr 8, 2026

Thank you for your contribution @NourhaneBoudaoud @ZHENG-Qi625

Please don't forget that your pull request won't be integrated if your commits they don't follow strict naming conventions.

It is quite easy with Git-Fork. See how it was done for a different issue: #390 (comment)
If you have any questions about these vidéos, just ask me.

Please note that the videos also show how Co-authored-by GitHub syntax should be used in commits.

@benel
Copy link
Copy Markdown
Member

benel commented Apr 22, 2026

@NourhaneBoudaoud @ZHENG-Qi625

Don't forget to fix your Git history as well as to fill your license term agreement.

@NourhaneBoudaoud NourhaneBoudaoud changed the title Update include_video.feature SCENARIO: Simplified Media Insertion (see #365). May 12, 2026
@NourhaneBoudaoud NourhaneBoudaoud requested a review from benel May 12, 2026 12:46
Co-authored-by: Nourhane BOUDAOUD nourhane.boudaoud@utt.fr
Co-authored-by: Qi ZHENG qi.zheng@utt.fr
Co-authored-by: Zhenpeng LI zhenpeng.li@utt.fr
Co-authored-by: Nourhane BOUDAOUD nourhane.boudaoud@utt.fr
Co-authored-by: Qi ZHENG qi.zheng@utt.fr
Co-authored-by: Zhenpeng LI zhenpeng.li@utt.fr
Copy link
Copy Markdown
Member

@benel benel left a comment

Choose a reason for hiding this comment

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

Thank you for the update, @NourhaneBoudaoud @ZHENG-Qi625

It is better than it was, but please have a look at my inline comments.
Don't forget also to put all your edits of jsx files in the same IMPROVEMENT commit.

Fonctionnalité: Insérer dans un document une vidéo
Fonctionnalité: Insérer dans un document une vidéo sur YouTube

Scénario: provenant de YouTube
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please don't change those 2 lines since we hope, one day, to be compatible also with Dailymotion, Pod, etc.


function FormattedText({children, setHighlightedText, selectable, setSelectedText}) {

function FormattedText({ children, setHighlightedText, selectable, setSelectedText }) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please never commit format change (like spaces), it breaks "git blame" and generates unnecessary edition conflicts when rebasing branch.

};

return (
<>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please never commit format change (like line break and indentation), it breaks "git blame" and generates unnecessary edition conflicts when rebasing branch, and make code review far more long and complex.

return (
<iframe width="80%" height="300" src={embedLink} frameBorder="0" allowFullScreen></iframe>
);
return <iframe width="80%" height="300" src={embedLink} style={{ border: 0 }} allowFullScreen title="YouTube video" />;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please never commit format change (like line break, parenthesis and indentation), it breaks "git blame" and generates unnecessary edition conflicts when rebasing branch, and make code review far more long and complex.

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

Labels

Good first issue Issue that is easy to test and fix for new team members or external contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplified Media Insertion (Video/Photo)

3 participants