openstore-ecommerce / OpenStore

OpenStore
25 stars 20 forks source link

Incorrect flags #200

Closed leedavi closed 3 years ago

leedavi commented 3 years ago

OpenStore uses it's own flags.

\DesktopModules\NBright\NBrightBuy\Themes\config\img\flags\16

This path should be changed to use the DNN flags.

leedavi commented 3 years ago

This was hiding all flag images in DNN9.x , so I reverted it.

Maybe we should create a token that uses the full img path.

\Website\images\Flags

with a width of "16px", I think.

DNNMonster commented 3 years ago

Token is good. I'll jump on this in the next few days unless you have this on a high priority list. Canna busy right now.

leedavi commented 3 years ago

OK great. :-)

DNNMonster commented 3 years ago

The default image sizes for DNN are 27 X 18px.

If we're not using one of the config theme images or the Dnn Image Handler then...

Inline style attributes on the flag images so we maintain the usage of the size params in our funcs?

Or

Add a "langflag" class name and use the backoffice.css? Then those size params could slated for deprecation. Cleanup may require removing a few non breaking relic code fragments related to the size (ie CreateEditCultureSelect).

Either of those changes would also let one remove the 3 flag image directories so we trim down a bit.

Pick your poison.

DNNMonster commented 3 years ago

There is branch you can review that goes the class name route and modifies the existing tokens.

Maybe you don't want to touch any of the existing flag related token code and introduce and entirely new one?

leedavi commented 3 years ago

I think that fix you've done is good. It looks to be working well on my test system.

Maybe this should be targeted at he next release v4.1.8. When you merge. Can you merge it to develop and master branch?

Changing the version to v4.1.8 could be part of the change?

What do you think about the redundant flag folders?
My thought is we can remove them from a new install. Any new installs will not have the flags. It may give a problem with some plugins, they will not have the new flag path, if they do not use the token. But they should be using the same tokens you have changed.

As for the relic code, I think we should leave it for now.

The "GenXmlTemplateExt.cs" class should be made redundant, but it is still in use for some things. Although all plugins should not use it now. Leaving redundant code doesn't hurt, just confuses. :-)