Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mm adding new profiles process #88

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

monicamcgibbon
Copy link
Contributor

No description provided.

@vaelliott
Copy link
Collaborator

looking good but i've not finished my review yet. You'll see I've pushed a couple of changes, one of which was to check what happens if we have a number of profiles that isn't a multiple of 3. It looks like we just get an empty area - something we may or may not want. I quite like the idea of suggesting whats next - i just dont' want to commit to a child poverty profiles just yet (until i'm sure we have resource to actually work on it).

@vaelliott
Copy link
Collaborator

I made a few tweaks to re-order profiles so they match the current live tool order and replaced a few 'and' with '&' to reduce the length of descriptions.
I also had to resolve a merge conflict which seemed pretty straight forward but when merging I noticed a number of files were updated which i'm guessing are branches that have been merged lately.

@monicamcgibbon
Copy link
Contributor Author

@abigap01 I forgot to add you as a reviewer for this. Could you please take a look and check if my comments make sense, and that you're able to follow the process for adding a new profile? (I'll email you the SOP now). You just need to look at the global, ui and server script and see if you can follow the comments and broadly make sense of how the new profiles list in the global script is being used to create the landing page buttons and to determine which tabs are hidden/shown for each profile. It would be really helpful to know if there is anything confusing at all, big or small so I can improve the comments for it. Can chat through at our catch up tomorrow too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants