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

Feat/startpage new arch #70

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ManagarmrWhisperer
Copy link
Contributor

No description provided.

@ManagarmrWhisperer ManagarmrWhisperer marked this pull request as draft March 14, 2024 10:07
@ManagarmrWhisperer ManagarmrWhisperer marked this pull request as ready for review March 14, 2024 22:26
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant new line.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still pending.

SPAN_TITLE_ELEMENT_SELECTOR = '.w-gl__result-title > h3, .result-title > h2 ';
BADGE_ELEMENT_SELECTOR = this.SPAN_TITLE_ELEMENT_SELECTOR; // Element that will hold the badge.
ANCHOR_ELEMENT_SELECTOR = '.w-gl__result-url, .css-1su0nhd > span, .css-1qvmgy0 > span'; // URL breadcumb
/**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing new lines between property and method.

ENGINE_LAYOUT_SELECTOR = '.w-gl--desktop, .w-gl';
RESULT_CONTAINER_SELECTOR = '.w-gl__result, .result';
SPAN_TITLE_ELEMENT_SELECTOR = '.w-gl__result-title > h3, .result-title > h2 ';
BADGE_ELEMENT_SELECTOR = this.SPAN_TITLE_ELEMENT_SELECTOR; // Element that will hold the badge.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Place the comment above the declaration as a block comment.

RESULT_CONTAINER_SELECTOR = '.w-gl__result, .result';
SPAN_TITLE_ELEMENT_SELECTOR = '.w-gl__result-title > h3, .result-title > h2 ';
BADGE_ELEMENT_SELECTOR = this.SPAN_TITLE_ELEMENT_SELECTOR; // Element that will hold the badge.
ANCHOR_ELEMENT_SELECTOR = '.w-gl__result-url, .css-1su0nhd > span, .css-1qvmgy0 > span'; // URL breadcumb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Place the comment above the declaration as a block comment.

class DdgSearchModule extends GenericSearchModule {
ENGINE_LAYOUT_SELECTOR = '.w-gl--desktop, .w-gl';
RESULT_CONTAINER_SELECTOR = '.w-gl__result, .result';
SPAN_TITLE_ELEMENT_SELECTOR = '.w-gl__result-title > h3, .result-title > h2 ';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant space at the end of the selector.

allMoved: true,
theme: {
fontSize: '80%',
color: document.documentElement.classList.contains("dark" || "startpage-html--night") ? '#a7b1fc' : '#000000',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use single quotes. Spaces before and after parentheses. What the hell is the condition anyway? That won't do what you think it will.

} );
containerElement.querySelector( this.BADGE_ELEMENT_SELECTOR ).appendChild( badgeElement );

//Rewrite URL breadcrumb
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing space after the comment starts. Please just install ESLint.

}
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Two lines after a class definition and before other code.

}
}

document.onreadystatechange = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

window.addEventListener, or awaitElement.

@@ -1,121 +0,0 @@
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please review your changes before you commit them. This should NOT had been deleted.

@alex4401 alex4401 added the feature New feature or request label Mar 16, 2024
@alex4401 alex4401 added this to the v1.9.0 milestone Mar 16, 2024
Copy link
Collaborator

@alex4401 alex4401 left a comment

Choose a reason for hiding this comment

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

I have no clue what changed since last review, but obviously there's still issues that are yet to be resolved, and all have been waiting since the initial review.

}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still pending.

},
{
id: 'ddg',
supportsFilter: true,
supportsRewrite: true,
supportsRewrite: true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've already told you there's no need for lines unrelated to the startpage change to be modified.

} );


class DdgSearchModule extends GenericSearchModule {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Class name.

}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant new lines.

if ( event.target.readyState === 'complete' ) {
DdgSearchModule.invoke( wikis );
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Redundant new line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants