Skip to content

[4.0] Remove duplicate scss lines for form stylings#33834

Closed
eyvazahmadzada wants to merge 2 commits intojoomla:4.0-devfrom
eyvazahmadzada:duplicate-styles
Closed

[4.0] Remove duplicate scss lines for form stylings#33834
eyvazahmadzada wants to merge 2 commits intojoomla:4.0-devfrom
eyvazahmadzada:duplicate-styles

Conversation

@eyvazahmadzada
Copy link
Copy Markdown

@eyvazahmadzada eyvazahmadzada commented May 13, 2021 •
edited
Loading

Pull Request for Issue #33829 .

Intro

Hello, I'm one of the applicants for GSoC 2021 for the Media Manager project and I'm glad to start contributing to Joomla by creating a pull request for this small issue.

Summary of Changes

Removed stylings from line 78-108 in /administrator/templates/atum/scss/blocks/_form.scss

Testing Instructions

Go to any page that contains a form (for example Administrator > Articles > New > Images and Links) and everything will work normally.

Actual result BEFORE applying this Pull Request

Duplicate scss code in /administrator/templates/atum/scss/vendor/bootstrap/_form.scss (from line 18-48) and in /administrator/templates/atum/scss/blocks/_form.scss (from line 78-108)

Expected result AFTER applying this Pull Request

No duplicate stylings

Documentation Changes Required

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels May 13, 2021
@ceford
Copy link
Copy Markdown
Contributor

ceford commented May 13, 2021

I have tested this item ✅ successfully on 9753763

To test I searched template.css before and after npm run build:css and saw the duplicate styles disappear.

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

@richard67 richard67 mentioned this pull request May 13, 2021
Closed
@YatharthVyas
Copy link
Copy Markdown
Contributor

YatharthVyas commented May 13, 2021

I have tested this item ✅ successfully on 9753763

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

@drmenzelit
Copy link
Copy Markdown
Collaborator

drmenzelit commented May 13, 2021

Thank you @eyvazahmadzada for your contribution. You need to check the classes before removing the duplicates: if a class is a Bootstrap class (e.g. form-check) a override of the class should be in the file administrator/templates/atum/scss/vendor/bootstrap/_form.scss, if the class is specific for Atum, the class should be in administrator/templates/atum/scss/blocks/_form.scss. At least that is the logic I would expect from the scss files in the template.

@eyvazahmadzada
Copy link
Copy Markdown
Author

eyvazahmadzada commented May 13, 2021 •
edited
Loading

Thank you @eyvazahmadzada for your contribution. You need to check the classes before removing the duplicates: if a class is a Bootstrap class (e.g. form-check) a override of the class should be in the file administrator/templates/atum/scss/vendor/bootstrap/_form.scss, if the class is specific for Atum, the class should be in administrator/templates/atum/scss/blocks/_form.scss. At least that is the logic I would expect from the scss files in the template.

Yes, you're right, that makes more sense. I've made a new commit to place stylings in their appropriate file.

@RickR2H
Copy link
Copy Markdown
Member

RickR2H commented Jun 24, 2021

I have tested this item ✅ successfully on e84d2d9

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

@eyvazahmadzada eyvazahmadzada deleted the duplicate-styles branch July 16, 2021 03:24
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

NPM Resource Changed This Pull Request can't be tested by Patchtester

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@eyvazahmadzada @ceford @YatharthVyas @drmenzelit @RickR2H @joomla-cms-bot