search

dddeastanglia / DDDEastAnglia

DDD East Anglia website
https://www.dddeastanglia.com
7 stars 10 forks source link

Fix #268 User pages in the admin section #270

Closed adrianbanks closed 10 years ago

adrianbanks commented 10 years ago

I've fixed the links, but also implemented:

alastairs commented 10 years ago

Hmmm, is there any possibility of pulling the two parts of this work out into separate pull requests...?

Why did you need to split Bootstrap out into a separate style bundle? Does this result in a separate CSS URL as well? I can't quite remember how the bundles are "rendered" in production.

adrianbanks commented 10 years ago

Hmmm, is there any possibility of pulling the two parts of this work out into separate pull requests...?

Which two parts are you referring to?

Why did you need to split Bootstrap out into a separate style bundle? Does this result in a separate CSS URL as well? I can't quite remember how the bundles are "rendered" in production.

Because bootstrap wasn't being included in the admin area layout page, and in the bundle it was bundled with the css files for the main site was there (which we don't use in the admin area). Having said that, I've just looked at the diffs and what I thought I'd done isn't what is here, so I'll tweak it tonight.

alastairs commented 10 years ago

I meant this bit:

I've fixed the links, but also implemented [other stuff]

It would be useful to have the bugfix in one pull request and the new features in a separate one.

adrianbanks commented 10 years ago

Didn't think it was worth separating the two, as fixing the links then means that the link urls are correct, but 404 when clicked on as there are no actions to handle them :confused:

alastairs commented 10 years ago

Oh! I hadn't appreciated that, sorry. No worries then; just the question about Bootstrap to resolve, and I'll do a quick review of the rest of the change now.

adrianbanks commented 10 years ago

I think I've sorted out the styles now - seems to look ok after the css shuffle.