Skip to content

Component config: Improve and fix code style, hidden field + Add style for fieldset description#10622

Merged
roland-d merged 2 commits intojoomla:stagingfrom
cyrez:Add-style-alert-info-for-fieldset-description-in-component-config
Jun 25, 2016
Merged

Component config: Improve and fix code style, hidden field + Add style for fieldset description#10622
roland-d merged 2 commits intojoomla:stagingfrom
cyrez:Add-style-alert-info-for-fieldset-description-in-component-config

Conversation

@cyrez
Copy link
Copy Markdown
Contributor

@cyrez cyrez commented May 25, 2016 •
edited by joomla-cms-bot
Loading

Summary of Changes

  • Fix code style
  • Fix hidden field (no html to be rendered when field is hidden, to prevent empty lines (due to css classes for fields)
  • Add style alert-info + icon for the description of a fieldset (description in top of each tab, when filled)

Testing Instructions

Test for fieldset description style:

  • Go to any component configuration page (Options) and check description has now an alert-info look

Before Patch:
capture d ecran 2016-05-24 a 01 07 00

After Patch:
capture d ecran 2016-05-24 a 01 07 30

Test for hidden fields fix:

  • To test fix for hidden fields, add in a config.xml those lines :
        <field
            name="hidden1"
            type="hidden"
            default="1"
        />

        <field
            name="hidden2"
            type="hidden"
            default="1"
        />

        <field
            name="hidden3"
            type="hidden"
            default="1"
        />

        <field
            name="hidden4"
            type="hidden"
            default="1"
        />

Before Patch:
capture d ecran 2016-05-25 a 12 47 05

After Patch:
capture d ecran 2016-05-25 a 12 47 37

  • You can control after save, that hidden fields are saved in the database (#__extensions > component params)

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented May 25, 2016

Actually, not related to your Pr, but i think that message is not correct.
I mean the category can also override article settings.

andrepereiradasilva
andrepereiradasilva reviewed May 25, 2016
View reviewed changes
administrator/components/com_config/view/component/tmpl/default.php

<!-- Begin Sidebar -->
<div id="sidebar" class="span2">
<div class="span2" id="sidebar">
Copy link
Copy Markdown
Contributor

@andrepereiradasilva andrepereiradasilva May 25, 2016

Choose a reason for hiding this comment

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

why did you change this? IMHO is better to have id, name and type attributes (when they exist) first.

Copy link
Copy Markdown
Contributor Author

@cyrez cyrez May 25, 2016

Choose a reason for hiding this comment

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

Code standards HTML : class, id, data-*, Everything else (https://joomla.github.io/coding-standards/?coding-standards/chapters/html.md)
IMO, BS 3 demo site use this order too ;-)

Copy link
Copy Markdown
Contributor

@andrepereiradasilva andrepereiradasilva May 25, 2016

Choose a reason for hiding this comment

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

ok then! my bad

@cyrez
Copy link
Copy Markdown
Contributor Author

cyrez commented May 25, 2016

Actually, not related to your Pr, but i think that message is not correct.
I mean the category can also override article settings.

Exactly: nothing related to this PR, as screenshot is just an example ;-)

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented May 25, 2016

you're right. i should have read the HTML coding standard before. It seems all ok.

@andrepereiradasilva
Copy link
Copy Markdown
Contributor

andrepereiradasilva commented May 25, 2016

I have tested this item ✅ successfully on 497e7db

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10622.

@cyrez
Copy link
Copy Markdown
Contributor Author

cyrez commented May 25, 2016

@andrepereiradasilva Thanks for testing!

you're right. i should have read the HTML coding standard before. It seems all ok.

Not exactly what i meant, but i asked to be sure you understand it as i did for HTML code standards, and so that we agree! ;-)

@ghost
Copy link
Copy Markdown

ghost commented Jun 25, 2016

I have tested this item ✅ successfully on 497e7db

Tested successfully

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10622.

@ghost
Copy link
Copy Markdown

ghost commented Jun 25, 2016

Tested successfully.

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10622.

@zero-24
Copy link
Copy Markdown
Contributor

zero-24 commented Jun 25, 2016

RTC based on testing

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10622.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 25, 2016
@roland-d roland-d added this to the Joomla 3.6.0 milestone Jun 25, 2016
@roland-d roland-d merged commit 7a9dca7 into joomla:staging Jun 25, 2016
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 25, 2016
@roland-d
Copy link
Copy Markdown
Contributor

roland-d commented Jun 25, 2016

Thanks everybody

This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10622.

@hacki65 hacki65 mentioned this pull request Oct 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

Joomla 3.6.0

Development

Successfully merging this pull request may close these issues.

5 participants

@cyrez @andrepereiradasilva @zero-24 @roland-d @joomla-cms-bot