Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

Recommended Posts

Just finished the checkout_confirmation products module, will PR soon. Just a couple of questions:

Does all the modules have the 1-to-12 content width options? I'm not sure if it has been standarized in any way. As a thought, the warning that many modules have on its description was useful but now looks useless to throw a warning on each module that has sizable options; and is also inside the description, that's not very logical. I'd wipe out those warnings - it's now repetitive to see them in every module.

Also, is there any reason for using classes instead of ids in the selectors of the template? I think this makes more sense:

  <div class="col-sm-<?php echo $content_width; ?>" id="cm-cc-products">

 

Link to comment
Share on other sites

  • Replies 213
  • Created
  • Last Reply

@raiwa @burt Just noticed that "const" was now being used in the newer language files instead of defines.  Is that what we should be doing for this project?

Dan

Link to comment
Share on other sites

Congratulations guys....subject to whatever tweaks might be necessary, it looks like the checkout_payment.php modules are now complete. :thumbsup:

Dan

Link to comment
Share on other sites

5 hours ago, Dan Cole said:

@raiwa @burt Just noticed that "const" was now being used in the newer language files instead of defines.  Is that what we should be doing for this project?

I remember that this was discussed in the modular shopping cart project.

Gary stated that both versions are fine. So no need to change anything.

Link to comment
Share on other sites

8 hours ago, piernas said:

Does all the modules have the 1-to-12 content width options? I'm not sure if it has been standarized in any way. As a thought, the warning that many modules have on its description was useful but now looks useless to throw a warning on each module that has sizable options; and is also inside the description, that's not very logical. I'd wipe out those warnings - it's now repetitive to see them in every module.

It looks repetitive for us, but still makes sense for new users. As it is included in all existing core content modules I would keep it.

Please do not change things without discussion and consense here. This project will be pushed to core, it is not an add-on.

Another thing would be to remove it in all modules and place it at the top of the content module page. But this is up to Gary.

8 hours ago, piernas said:

Also, is there any reason for using classes instead of ids in the selectors of the template? I think this makes more sense:


  <div class="col-sm-<?php echo $content_width; ?>" id="cm-cc-products">

 

Same as before, class is used in all core modules and does the job. I see no reason to change it.

 

@Dan Cole, @burt, your opinion??

Link to comment
Share on other sites

@cupidare, Our project has nothing to do with the content of the address blocks. We only move what already exists into modules.

What you ask for would be a store customization which requires database table changes etc.

Link to comment
Share on other sites

Another point:

The list of content modules in admin=>modules=>content is growing up fast.

How about to organize the content modules by (page) groups in tabs or something similar.

@piernas published an add-on which does this since time on the marketplace:

Improved content modules Admin for 2.3.4

@burt, something for core?

Link to comment
Share on other sites

4 hours ago, raiwa said:

@Dan Cole, @burt, your opinion??

I agree...IMO, all we should be doing is taking what is and turning it into a modular version.  The only thing I would take on board are things that Gary would want changed or added to better fit the current standard being applied to the core package.

Dan

Link to comment
Share on other sites

const vs define
I prefer const as IMO it is easier to read and code.

id vs class
class is a bit more useful as can be used multiple times per page, id can only be used one time per page

warning about width
keep in modules

different admin page for modules
this has come up in an email conversation, and agreed, should be moved into core. 
@piernas is that addon solid as it stands now or does it need looking at again?

Link to comment
Share on other sites

Link to comment
Share on other sites

Lets think about that...another day.

Tabs could easily be changed to a dropdown.
Or have the 5 "most important" tabs, then a dropdown for the rest.
Or something else. 

Let's get the first piece of the puzzle in place, then think about the rest of the puzzle.

Link to comment
Share on other sites

@wHiTeHaT yes probably four or five rows of tabs, but for now it's only two.

Here's the modified file @raiwa @Dan Cole @burt It works nicely here, please test and post your comments.

:modules_content.rar

I've renamed it to modules_content2.php to simplify testing.

This is how it looks:

modules_content_admin.thumb.png.1b01b042eb42853e076b038b045de3a8.png

Had a version that allowed to turn on/off modules and to assign sort order by pressing buttons, but was using some tricks to figure out constant names so I removed it from the file.

Also had to use

        $description = preg_replace('(<div\s+class="secWarning">.*?<\/div>)', '', $module->description);
        preg_match('(<div\s+class="secWarning">.*?<\/div>)', $module->description, $warning);

to strip off the warnings from description and add them again separately. Would be cleaner if a new warning property was added to classes instead of description.

Plus, used a new function to adapt the output of tep_cfg_select_option.

Please test and give feedback.

 

Link to comment
Share on other sites

new point to discuss:

old "checkout_new_address.php" module:

  • call in modules via "include": +one module for 2 pages, less work to customize
    OR
  • move content into new modules. +up to date with current coding standards
Link to comment
Share on other sites

@ArtcoInc,

I have plans to update all my add-ons to the new code base once BS Final will be released.

And I have some new add-ons in process. :wink:

Link to comment
Share on other sites

3 hours ago, raiwa said:

new point to discuss:

old "checkout_new_address.php" module:

  • call in modules via "include": +one module for 2 pages, less work to customize
    OR
  • move content into new modules. +up to date with current coding standards

Is this an existing module or something you're proposing to do?  I'm confused as usual. :wacko:

Dan

Link to comment
Share on other sites

10 hours ago, Dan Cole said:

Is this an existing module or something you're proposing to do?  I'm confused as usual. :wacko:

Dan

I refer to the existing file: includes/modules/checkout_new_address.php

It contains the new address input fields which are used in checkout_shipping_address.php and checkout_payment_address.php.

It is called in both pages via "include"

Link to comment
Share on other sites

14 hours ago, Dan Cole said:

call in modules via "include": +one module for 2 pages, less work to customize

I would just keep it simple and leave it as is.  It can always be dealt with later and doesn't really impact what we're doing. 

Dan

Link to comment
Share on other sites

@Dan Cole, @piernas, @burt,

I was playing with the modules in checkout shipping. It seems the clearfix divs which have been moved from the main page into the shipping methods and step wizard modules are not needed any more.

I tried with different sort orders and module widths.

There is no difference how the page renders, with and without.

I believe beeing wrapped the content now in its own col div, the clearfix div is obsolete.

Can you confirm this?

Same for all clearfix divs in all other pages, I guess.

Link to comment
Share on other sites

I haven't tested extensively but I did play around with it when I was putting the step wizards together and it didn't seem to make a difference that I could see.

Dan

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...