Problem/Motivation
we have to many css floating around that dont do much more than add a bit of css to seven
Its confusing for everybody why & where it comes from so lets clean it up
File.admin.css dont contains anything that makes it critacal for it to function, its pure layout
/* File upload widget.*/
.form-managed-file .form-submit {
margin: 0 0.5em;
}
.form-managed-file div.ajax-progress-bar {
display: none;
margin-top: 4px;
padding: 0;
width: 28em;
}
.form-managed-file .ajax-progress-bar .bar {
margin: 0;
}
Proposed resolution
move the css over to seven, where it belongs
Remaining tasks
User interface changes
API changes
Comments
Comment #1
rudraram commentedMoved css into seven as proposed above.
Comment #2
rudraram commentedApologies. My patch above didn't capture all the changes. Updated the above patch.
Comment #3
chernous_dn commentedComment #4
chernous_dn commentedUpdate patch #2. Rename of file
file.admin.csstoform-managed-file.css. Match the class styled within. Addform-managed-file.csstoseven.libraries.yml. Removed reference in filefile.libraries.yml. Add screenshots.Comment #5
mortendk commented#5 Renaming css files it out of scope with this issue, lets not make things more complicated. We must first the css files over to the theme (classy or in this case seven) then take the discussion of what we want to name them.
Comment #6
chernous_dn commentedHI @mortendk in issue https://www.drupal.org/node/2491259 @LewisNyman recommended renaming file:
I decided to do it.
Comment #7
mortendk commentedthe problem by renaming files as "we feel like it" with no cencensus, is were gonna end up with the mess.
If we rename files as we go lets keep them as simple & desciptive as possible in this case
form-file.cssSeven is naming files to be on a component level, afaik its not gonna be named after a class (that somebody came up with 8 years ago)
.form-managed-fileWhat we agreed upon in LA at the meta discussion was to move all css files that we could move - then after that do the work of naming & placing the css files the right places.
Anyways lets not bikeshed this keep it simple as possible and get the stuff moved in, cause were running out of time...
Comment #8
lewisnyman@mortendk All the CSS files in Seven are named after the components within them, the classes are the component names. If you want to put this in Seven then you need to follow the conventions in the theme. Anyway you've already renamed the file from file.admin.css so what's the harm? We can bikeshed the real class name in another issue.
Comment #9
mortendk commentedupsie ;) lemme just reroll this *coughs* nothing to see here
Comment #10
mortendk commentedreuploading the patch from #4
*whisteling*
Comment #11
lewisnymanReuploading as .patch
Comment #12
lewisnymanThis is hard to test because a lot of this CSS affects the
ajax-progress-barclass and I can't get my local server set up to display the filter upload progress.Is anyone able to do this and take screenshots before/after?
Comment #13
chernous_dn commented@LewisNyman I add screenshots in #4 comment.
Comment #14
lewisnyman@Chernous_dn Is it possible for you to get of the upload progress bar?
Comment #15
chernous_dn commented@LewisNyman Yes sure, add screenshots. I tested in file filed, the same result.
Comment #16
lewisnymanTop work! Thank you
Comment #17
alexpottI think that this needs to go Bartik too as the node form which can contain file fields can appear in bartik and same with the account forms since peeps can add file field to that too.
Comment #18
lewisnymanIn that case, we move it to Classy
Comment #19
pjbaertI moved the File.admin.css to the classy css/components folder & as I decided to use the same analogy @LewisNyman mentioned in https://www.drupal.org/node/2489566#comment-10216757
Comment #20
chernous_dn commented@pjbaert I already create patch for Classy, we need create patch for Bartik.
Comment #21
chernous_dn commentedUpdate patch, add style to Bartik theme.
Comment #22
lewisnymanI'm confused, the patch in #19 is adding the file to Classy, and the patch in #21 is adding the same file to Bartik and Seven.
@Chernous_dn Is this what you intended? If we add the file to Classy then it will already be loaded in Bartik and Seven.
Comment #24
aadrian commentedrerolled
Comment #27
chernous_dn commentedReuploading the patch from #19 I think this is the correct patch.
Comment #29
mayurjadhav commentedI think this patch will work.
Comment #30
joginderpcManual tested and verified file is moved to classy theme. screen shot attached below.
Comment #31
alexpottWhen changing core css files, they need to be moved to stable. Perhaps when this issue was created stable did not exist. Classy extends stable.
Comment #32
chernous_dn commentedUpdate my old patch #4 rename and move to classy css file.
Comment #33
chernous_dn commented#alexpott In stable
core\themes\stable\css\file\file.admin.cssalready exist this file.Comment #34
alexpott@Chernous_dn slo then moving to classy is not necessary. SO just remove from core and fix the library in stable. We shouldn't need to change classy in this patch.
Comment #36
chernous_dn commentedRemove from core module file
file.admin.css..Comment #37
mayurjadhav commented@alexpott, do we need to create "stable.libraries.yml" in stable? else Chernous_dn's patch looks perfect.
Comment #38
chernous_dn commentedHi @mayurjadhav if I'm not mistaken
file.admin.cssinclude instable.info.ymlattach screenshot.Comment #39
mayurjadhav commentedYea, correct. Patch looks perfect for me now.
Comment #40
manjit.singhchanges in #36 looks good to me :) moving it to RTBC.
Comment #41
catchCommitted cecc8f7 and pushed to 8.2.x. Thanks!