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.

Sign up for GitHub

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

Jump to bottom

table widget #2

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

table widget #2

wants to merge 7 commits into from
+331 −6

Conversation

Mchris-hub
Copy link

PR Checklist

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Summary

This PR can be summarized in the following changelog entry:

Description

An explanation of what is done in this PR

Test instructions

This PR can be tested by following these steps:

Quality assurance

  • I have tested this code to the best of my abilities
  • I have added unittests to verify the code works as intended
  • Docs have been added / updated (for bug fixes / features)

Fixes #

@Mchris-hub Mchris-hub marked this pull request as draft February 26, 2024 14:45
hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
*
* @since 1.0.0
* @access public
*
* @return string Widget title.
*/
public function get_title() {
return esc_html__( 'testOne', 'elementor' );
return esc_html__( 'table', 'elementor' );
Copy link
Owner

Choose a reason for hiding this comment

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

I think we always use capital letters here:

return esc_html__( 'Table', 'elementor' );

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
@@ -86,7 +85,7 @@ public function get_categories() {
* @return array Widget keywords.
*/
public function get_keywords() {
return [ 'testOne', 'title', 'text' ];
return [ 'table', 'title', 'text' ];
Copy link
Owner

Choose a reason for hiding this comment

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

return [ 'table' ];

Copy link
Author

Choose a reason for hiding this comment

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

I removed the 'title' and the 'text'

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
@@ -104,14 +103,14 @@ protected function get_upsale_data() {
'condition' => ! Utils::has_pro(),
'image' => esc_url( ELEMENTOR_ASSETS_URL . 'images/go-pro.svg' ),
'image_alt' => esc_attr__( 'Upgrade', 'elementor' ),
'description' => esc_html__( 'Create captivating testOnes that rotate with the Animated Headline Widget.', 'elementor' ),
'upgrade_url' => esc_url( 'https://go.elementor.com/go-pro-testOne-widget/' ),
'description' => esc_html__( 'Create captivating tables that rotate with the Animated Headline Widget.', 'elementor' ),
Copy link
Owner

Choose a reason for hiding this comment

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

'description' => esc_html__( 'Create captivating tables.', 'elementor' ),

Copy link
Author

Choose a reason for hiding this comment

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

I changed the description

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
package.json Outdated
@@ -75,7 +75,7 @@
"eslint-plugin-no-jquery": "^2.7.0",
"eslint-plugin-react": "^7.33.2",
"eslint-webpack-plugin": "^4.0.1",
"grunt": "^1.5.3",
"grunt": "^1.6.1",
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert to the orginal value. We can't update packages on the whole repository.

Copy link
Author

Choose a reason for hiding this comment

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

I reverted

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
package-lock.json Outdated
@@ -85,7 +85,7 @@
"eslint-plugin-no-jquery": "^2.7.0",
"eslint-plugin-react": "^7.33.2",
"eslint-webpack-plugin": "^4.0.1",
"grunt": "^1.5.3",
"grunt": "^1.6.1",
Copy link
Owner

Choose a reason for hiding this comment

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

Revert changes. We can't make updates here.

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
package.json Outdated
@@ -75,7 +75,7 @@
"eslint-plugin-no-jquery": "^2.7.0",
"eslint-plugin-react": "^7.33.2",
"eslint-webpack-plugin": "^4.0.1",
"grunt": "^1.5.3",
"grunt": "^1.6.1",
Copy link
Owner

Choose a reason for hiding this comment

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

Revert changes. We can't make updates here.

Copy link
Author

Choose a reason for hiding this comment

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

i changed it

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated

<# if ( settings.list.length ) { #>
<tbody>
<tr>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<tr>

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
<# if ( settings.list.length ) { #>
<tbody>
<tr>
<# _.each( settings.list, function( item ) { #>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<# _.each( settings.list, function( item ) { #>
<# _.each( settings.list, function( item ) { #>
<tr>

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
<tr>
<# _.each( settings.list, function( item ) { #>
<td class="elementor-repeater-item-{{ item._id }}">{{{item.list_title}}}</td>
<# }); #>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<# }); #>

hein-obox
hein-obox reviewed Apr 5, 2024
View reviewed changes
includes/widgets/table.php Outdated
<# _.each( settings.list, function( item ) { #>
<td class="elementor-repeater-item-{{ item._id }}">{{{item.list_title}}}</td>
<# }); #>
</tr>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
</tr>
</tr>
<# }); #>

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
$this->add_control(
'rows',
[
'label' => esc_html__( "Rows' contents", 'elementor' ),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
'label' => esc_html__( "Rows' contents", 'elementor' ),
'label' => esc_html__( 'Content rows', 'elementor' ),

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
'rows',
[
'label' => esc_html__( "Rows' contents", 'elementor' ),
'show_label' => esc_html__( true, 'elementor' ),
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
'show_label' => esc_html__( true, 'elementor' ),
'show_label' => true,

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
'name' => 'column_content_one',
'label' => esc_html__( 'Content column 1', 'elementor' ),
'type' => \Elementor\Controls_Manager::TEXT,
'default' => esc_html__( 'Content column 1' , 'elementor' ),
Copy link
Owner

Choose a reason for hiding this comment

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

Remove default values.

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
if ( $settings['rows'] ) {
echo '<tbody>';
$rows = $settings['rows'];
for ($i=0; $i < count($rows) - 1; $i++) {
Copy link
Owner

Choose a reason for hiding this comment

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

I think the rows minus 1 is causing a problem.

Elementor used always spaces in between, check the other files in the repository.

Suggested change
for ($i=0; $i < count($rows) - 1; $i++) {
for ( $i=0; $i < count( $rows ); $i++ ) {

Copy link
Author

Choose a reason for hiding this comment

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

yes I should have put <= like in the JS side

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
<# if ( settings.rows.length ) { #>
<tbody>
<# let rows = settings.rows #>
<# for(let i = 0; i <= rows.length - 1; i++){ #>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<# for(let i = 0; i <= rows.length - 1; i++){ #>
<# for( let i = 0; i <= rows.length - 1; i++ ){ #>

Copy link
Author

Choose a reason for hiding this comment

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

i changed it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
<# let rows = settings.rows #>
<# for(let i = 0; i <= rows.length - 1; i++){ #>
<tr>
<td class="elementor-repeater-item-{{ rows[i]._id }}">{{{rows[i].column_content_one}}}</td>
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
<td class="elementor-repeater-item-{{ rows[i]._id }}">{{{rows[i].column_content_one}}}</td>
<td class="elementor-repeater-item-{{ rows[i]._id }}">{{{ rows[i].column_content_one }}}</td>

Copy link
Author

Choose a reason for hiding this comment

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

I changed it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
includes/widgets/table.php Outdated
*
* @return array Widget promotion data.
*/
protected function get_upsale_data() {
Copy link
Owner

Choose a reason for hiding this comment

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

Delete?

Copy link
Author

@Mchris-hub Mchris-hub Apr 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

i deleted it

hein-obox
hein-obox reviewed Apr 11, 2024
View reviewed changes
package-lock.json
@@ -7447,9 +7447,9 @@
}
},
"node_modules/caniuse-lite": {
"version": "1.0.30001532",
Copy link
Owner

Choose a reason for hiding this comment

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

Revert changes.

hein-obox
hein-obox reviewed Apr 12, 2024
View reviewed changes
includes/widgets/table.php
$this->add_control(
'table_head_color',
[
'label' => esc_html__( 'Table head Color', 'elementor' ),
Copy link
Owner

Choose a reason for hiding this comment

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

Use labels from Trello.

Using the correct lowercase vs uppercase is important. You can't mix it randomly.

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

@hein-obox hein-obox hein-obox left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mchris-hub @hein-obox