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

Left Arrow functionality to the left arrow in beginner block #3531

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

kaurjasleen240305
Copy link
Contributor

Related to Issue :#3481

@kaurjasleen240305
Copy link
Contributor Author

pls review @walterbender

@kaurjasleen240305
Copy link
Contributor Author

kaurjasleen240305 commented Jan 7, 2024

pls review @samyakshah3008 @walterbender sir I have added what was told by you that we should go back to the tour on click of left arrow of beginneer block tour

@kaurjasleen240305 kaurjasleen240305 changed the title Added disable class to left arrow in blocks tour which was not there before Left Arrow functionality to the left arrow in beginner block Jan 7, 2024
@walterbender
Copy link
Member

Shouldn''t it go to the end of the tour rather than the start of the tour?

@kaurjasleen240305
Copy link
Contributor Author

kaurjasleen240305 commented Jan 7, 2024

@walterbender done sir pls review now onclicking the left arrow of beginner's block we are going to last page only of the overall tour.Also I have added heading changing functionality in overall tour where upon page changing heading also changes as in blocks tour

Copy link
Contributor

@samyakshah3008 samyakshah3008 left a comment

Choose a reason for hiding this comment

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

Code looks fine to me, but that would be great if you could record a video of new implementation of the functionality for better review experience. Or some screenshots.

@kaurjasleen240305
Copy link
Contributor Author

Done sir @walterbender @samyakshah3008 pls review

Music.Blocks.is.a.collection.of.tools.for.exploring.fundamental.musical.concepts.in.a.fun.way.-.Google.Chrome.2024-01-07.21-08-58.mp4

Copy link
Contributor

@samyakshah3008 samyakshah3008 left a comment

Choose a reason for hiding this comment

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

Nice efforts @kaurjasleen240305

@kaurjasleen240305
Copy link
Contributor Author

Thnx sir review of @walterbender is left yet .

@@ -69,9 +69,8 @@ class HelpWidget {
* @param {useActiveBlock} Show help for the active block.
* @returns {void}
*/
_setup(useActiveBlock) {
_setup(useActiveBlock,page) {
Copy link
Member

Choose a reason for hiding this comment

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

how about a space after the comma?

@@ -92,7 +91,12 @@ class HelpWidget {

let leftArrow, rightArrow;
if (!useActiveBlock) {
this.widgetWindow.updateTitle(_("Take a tour"));
if(page==0){
Copy link
Member

Choose a reason for hiding this comment

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

spaces around operators and before {s

@@ -411,19 +449,20 @@ class HelpWidget {
let cell = docById("right-arrow");
let rightArrow = docById("right-arrow");
let leftArrow = docById("left-arrow");

Copy link
Member

Choose a reason for hiding this comment

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

do we need this newline?


if(this.index == this.appendedBlockList.length - 1)
{
rightArrow.classList.add("disabled") ;       
        }
rightArrow.classList.add("disabled") ;
Copy link
Member

Choose a reason for hiding this comment

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

Since you are cleaning up formatting, please remove the space before the ;

@kaurjasleen240305 kaurjasleen240305 force-pushed the br4 branch 2 times, most recently from cc9fc12 to 6520a6d Compare January 7, 2024 18:57
@kaurjasleen240305
Copy link
Contributor Author

Sorry for inconvenience sir @walterbender but now formatting changes have been done.. you can check..

@kaurjasleen240305
Copy link
Contributor Author

pls review @walterbender

@@ -58,7 +58,7 @@ class HelpWidget {
this._helpDiv = document.createElement("div");

// Give the DOM time to create the div.
setTimeout(() => this._setup(useActiveBlock), 0);
setTimeout(() => this._setup(useActiveBlock,0), 0);
Copy link
Member

Choose a reason for hiding this comment

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

please add space after all commas.

@@ -117,6 +121,12 @@ class HelpWidget {
if (page > 0){
page = page - 1;
leftArrow.classList.remove('disabled');
if (page==0) {
Copy link
Member

Choose a reason for hiding this comment

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

Please add spaces around all operators.

@kaurjasleen240305 kaurjasleen240305 force-pushed the br4 branch 2 times, most recently from 7fa05aa to 7e8033b Compare January 7, 2024 19:48
added_disabled_property_to_blockHelp_div

one_more

one_more

block_left_arrow_active

left_arrow_function_to_last_page

formatting changes

formatting changes

spaces_operators

spaces_commans

spaces_comman

spaces_comman

operator_spaces
@kaurjasleen240305
Copy link
Contributor Author

kaurjasleen240305 commented Jan 8, 2024

Pls review @walterbender sir spaces in operator added ..

@walterbender walterbender merged commit be8c8c3 into sugarlabs:master Jan 8, 2024
3 checks passed
Mubashirshariq pushed a commit to Mubashirshariq/musicblocks that referenced this pull request Jan 10, 2024
added_disabled_property_to_blockHelp_div

one_more

one_more

block_left_arrow_active

left_arrow_function_to_last_page

formatting changes

formatting changes

spaces_operators

spaces_commans

spaces_comman

spaces_comman

operator_spaces
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