Skip to content

Update joomla-image-select.css#32728

Merged
drmenzelit merged 12 commits intojoomla:4.0-devfrom
hitesh-coder:4.0-dev
Mar 31, 2021
Merged

Update joomla-image-select.css#32728
drmenzelit merged 12 commits intojoomla:4.0-devfrom
hitesh-coder:4.0-dev

Conversation

@hitesh-coder
Copy link
Contributor

@hitesh-coder hitesh-coder commented Mar 18, 2021

Pull Request for Issue #31909.

Steps to reproduce

Content > Article
create a new article
click on cms-content dropdown select image
Click on any image additional data should popup

Summary of Changes

added a different background color and also added padding and border-radius which i thinks looks good

Expected result AFTER applying this Pull Request

This should make the necessary changes required for the desired styling

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Mar 18, 2021
@infograf768
Copy link
Member

Please post screenshots Before and After pr.

@hitesh-coder
Copy link
Contributor Author

hitesh-coder commented Mar 18, 2021

Before

Screenshot 2021-03-18 at 5 59 36 PM

After

Screenshot 2021-03-18 at 4 11 41 PM

@infograf768

@dgrammatiko
Copy link
Contributor

Can you please also style the summary element (to look like a button or an accordion header)?

@hitesh-coder
Copy link
Contributor Author

Updated as you asked @dgrammatiko

#BEFORE
Before

#AFTER
Screenshot 2021-03-19 at 3 26 28 PM

@drmenzelit
Copy link
Collaborator

Can you use the same border-radius as in other places on template?

You have to check your PR, there are more changed files as needed. And the tests are failing (you can click on "Details" to see what is causing the problem)
grafik

@drmenzelit
Copy link
Collaborator

The css files should not be commited

@dgrammatiko
Copy link
Contributor

@hitesh-coder
Copy link
Contributor Author

@dgrammatiko but you asked to change summary element as accordion header and that should be done in respective file
so should i now revert all my changes and apply as before

@brianteeman
Copy link
Contributor

@hitesh-coder the problem is that your change is global and not just on this one element

@hitesh-coder
Copy link
Contributor Author

AFTER

Screenshot 2021-03-19 at 7 50 16 PM

@rdeutz rdeutz added this to the Joomla 4.0.1 milestone Mar 22, 2021
@ceford
Copy link
Contributor

ceford commented Mar 28, 2021

I have tested this item ✅ successfully on b5f790a

Better with this styling. Should it have a border?


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

@meinhoonharsh
Copy link

I have not tested this item.

I tried to Test this, but when I Apply Patch, I can't see any change.


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

@drmenzelit
Copy link
Collaborator

@meinhoonharsh you can't test this using the patch tester, because the PR changes a scss file and for this you need to run npm run build:css to work

@hitesh-coder
Copy link
Contributor Author

hey @ceford thanks for testing but could you please elaborate on the thing you mention I would love to make it better

@infograf768
Copy link
Member

@hitesh-coder

add a border

joomla-field-mediamore details {
    position: absolute;
    background: #f5f5f5;
    border-radius: .25rem;
    border: 1px solid #c9c9c9; // add this
}

to get

Screen Shot 2021-03-29 at 09 14 35

@meinhoonharsh
Copy link

@meinhoonharsh you can't test this using the patch tester, because the PR changes a scss file and for this you need to run npm run build:css to work

Thank You @drmenzelit , I will make sure to do this from now

@meinhoonharsh
Copy link

meinhoonharsh commented Mar 29, 2021

I have tested this item ✅ successfully on b5f790a

Before:

image

After:

image

@hitesh-coder
Copy link
Contributor Author

hey @meinhoonharsh I have also added a border in my latest commit

@ceford
Copy link
Contributor

ceford commented Mar 29, 2021

I have tested this item ✅ successfully on 934c082

Yes - really nice with a border!


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

@meinhoonharsh
Copy link

I have tested this item ✅ successfully on 934c082

I have Successfully tested this. It looks good in order to comparision with previous one


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

@joomla-cms-bot joomla-cms-bot removed this from the Joomla 4.0.1 milestone Mar 30, 2021
@infograf768
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Mar 30, 2021
@infograf768 infograf768 added this to the Joomla 4.0 milestone Mar 30, 2021
@hitesh-coder hitesh-coder deleted the branch joomla:4.0-dev March 31, 2021 11:46
@hitesh-coder hitesh-coder deleted the 4.0-dev branch March 31, 2021 11:46
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 31, 2021
@drmenzelit
Copy link
Collaborator

@hitesh-coder why did you close this PR?

@hitesh-coder
Copy link
Contributor Author

@drmenzelit I mistakenly deleted the branch while exploring GitHub feature. can I make another PR?

@richard67 richard67 reopened this Mar 31, 2021
@richard67
Copy link
Member

@hitesh-coder No need to make a new PR. I've re-opened this so it can be merged. This can be done even when you branch was deleted, as far as I know.

@richard67 richard67 added the RTC This Pull Request is Ready To Commit label Mar 31, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 31, 2021
@drmenzelit drmenzelit merged commit 8c1a58d into joomla:4.0-dev Mar 31, 2021
@drmenzelit
Copy link
Collaborator

Thanks!

dgrammatiko pushed a commit to dgrammatiko/joomla-cms that referenced this pull request Apr 2, 2021
* Update joomla-image-select.css

* Update global.scss and joomla-image-select.css

* Revert "Update global.scss and joomla-image-select.css"

This reverts commit c73ef1d.

* Style are made as required

* adding good code practice

Co-authored-by: Quy <quy@fluxbb.org>

* adding good code practice

* This makes the necessary changes required joomla#32797

* Revert "This makes the necessary changes required joomla#32797"

This reverts commit 8a5e982.

* This make neccessary changes required

* Revert "This make neccessary changes required"

This reverts commit 68c7c41.

* Adding border

Co-authored-by: Quy <quy@fluxbb.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.