fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

Fas 3.0 fedorabootstrap #225

Closed ryanlerch closed 7 years ago

ryanlerch commented 7 years ago

Here is the initial work of converting FAS3 to the new fedorabootstrap style that we want to use going forward for fedora apps.

There are a handful of pages that aren't fully converted yet, but i would love to get this into the FAS3 branch so we can all hack on it going forward.

pypingou commented 7 years ago

For this kind of change, due to the amount and nature of the change, I'd favor testing it locally and just merge. We can then open tickets for fixing the remaining issues.

laxathom commented 7 years ago

So, I looked at your first integration and here's what I would like to propose from the amount a changes from the template:

Move the current theme as the :v: upstream :v: default's theme/template and use fedorabootstrap as fedoraproject's one.

We can also move your work in a dedicated git repo (fas-theme-fedoraproject) which will track fas as a sub-module to make sure the template/theme match with the right code change.

This also will provide a better work streak on the UX allowing it to have it's own life cycle (like any third partie which would like to build their own) as it can evolve w/o backend's code.

If you guys Ok with this, I will then move currect theme as default's one and adapt rpm subpackage -theme-fedoraproject accordingly. Provide a dedicated git with the necessary tools to build up the UX.

ryanlerch commented 7 years ago

i have no strong opinions on the repo layout, so we can do what you think is necessary here.

ryanlerch commented 7 years ago

I have started the seperation of themes here in this PR:

https://github.com/fedora-infra/fas/pull/227

Looking into this, not sure that git submodules are worth the hassle here. If we have clear seperation between the themes, i think it can all live in the same repo.

once we have this sorted out, i can go ahead updating this PR to add a new theme.

laxathom commented 7 years ago

If we have clear seperation between the themes, i think it can all live in the same repo.

works for me, as long as the related theme is versioned :+1:

ryanlerch commented 7 years ago

Updated this PR so it now works with the new theme directory layout

ryanlerch commented 7 years ago

Now these themes are separated out, are we right to merge this, and then keep hacking on the theme?

ryanlerch commented 7 years ago

It is purely for cleaning up that area, and putting the things that aren't needed all the time in the dropdown.

Also, the user is an admin, so it does fit in the user menu imho. Also it is clearly labeled fas admin settings to avoid confusion.

laxathom commented 7 years ago

hm..I'm still not convinced. The settings page can be used as much as group or people's by admin. As a navigation bar I expect to find settings navigation there.

puiterwijk commented 7 years ago

I think that this PR is in a reasonable state to merge and then finetune things in later PRs. That way, people can get started, and we will get feedback from other people too since they'll start seeing it,

laxathom commented 7 years ago

wfm