feat: restyle with brand colors

arrocke commented 9 months ago

What has changed

all views rebuilt to use new branding
admin views and language management views rebuilt with sidebar menu
user role can now be set when inviting user to the platform

There will undoubtedly be issues since this is so large, but to prevent ones that would continue to bloat this PR, let's only address critical functionality issues and open issues for the others. I'm going to include in that cleaning things up for mobile screens as well.

Connected Issues

closes #331

QA Steps

This touches pretty much everything, so you'll want to try all of the pages.

Post-Deployment

[x] Nothing required

Pertempto commented 9 months ago

Something seems off about the sidebar background color - https://github.com/arrocke/gloss-translation/issues/348

Pertempto commented 9 months ago

Is the manage button supposed to be lowercase like this?

Pertempto commented 9 months ago

Overall the new styling looks great! I really like the new layout for the settings pages.

I'm not trying to tear your changes apart when I create issues about how we could improve some small things.

Pertempto commented 9 months ago

Wow the create user page looks really nice! 🤩

Pertempto commented 9 months ago

Core Tests

[x] Create new user
[x] Create new language
[x] Update language settings
[x] Add users to language
[x] User access to languages
[x] Enter notes in translation sidebar
[x] Input gloss
[x] Approve gloss
[x] Revoke approval of gloss

@arrocke anything else that would be considered core functionality that I should test?

Pertempto commented 9 months ago

I have noticed a bug where when you add a new item to a list, that list does not update until you refresh the page.

It happened when I added a language and when I added a user to a language.

I think this would be important enough functionality to justify fixing it in this PR.

Pertempto commented 9 months ago

Something trivial haha... This is really more of a feat: than a refactor: A true refactor doesn't affect functionality. These changes DO impact functionality in a big way.

arrocke commented 9 months ago

@arrocke anything else that would be considered core functionality that I should test?

I think that's a good list

Something trivial haha... This is really more of a feat: than a refactor: A true refactor doesn't affect functionality. These changes DO impact functionality in a big way.

I think we can leave it as feat because the user can't really do anything new, they just do it a bit differently now.

Is the manage button supposed to be lowercase like this?

Fixed

I have noticed a bug where when you add a new item to a list, that list does not update until you refresh the page.

This is fixed as well

Pertempto commented 9 months ago

I think we can leave it as feat because the user can't really do anything new, they just do it a bit differently now.

Alright, for future reference how should we determining the types? I usually go by this list but you seem to be using different definitions. I guess I had in my brain that a refactor is something that wouldn't be worth a bump in version number because the user notices no differences.

arrocke commented 9 months ago

I did some more research on conventional commits and I think I was wrong. Seems like most people use refactor as you described. Users shouldn't see any difference because only the code has changed to produce the same result for users. I changed it to feat.

Pertempto commented 9 months ago

I really really like the new settings UI! 🤩 The auto-save is especially nice

The one thing I noticed is that the LTR indicator doesn't highlight the currently-selected value. Do you want to fix that here or would you like me to create a separate issue?

Pertempto commented 9 months ago

How do I view the landing page in my local env? I ran nx serve landing but I'm not sure what port on localhost it is using.

Pertempto commented 9 months ago

How do I view the landing page in my local env? I ran nx serve landing but I'm not sure what port on localhost it is using.

I opened index.html in my browser and that did it. Is that the correct way?

arrocke commented 9 months ago

The one thing I noticed is that the LTR indicator doesn't highlight the currently-selected value.

I'm not seeing this issue. Does it still occur when you reset your data seed?

How does this item not have to have the path field, but the first item (at line 19) does have a path field?

I think the path '/' on the first route is redundant. Both root routes are considered layout routes because they render nested routes inside of layout components. The AdminView route works similarly, but it prefixes that path with 'admin'.

Pertempto commented 9 months ago

I'm not seeing this issue. Does it still occur when you reset your data seed?

Yes. I reset the DB and still see the bug. Here is the response I'm getting from the API, which seems good.

Pertempto commented 9 months ago

Clicking the button send an update to the backend with the new textDirection. So the API and logic is working correctly, the UI just isn't displaying the currently selected value.

Pertempto commented 9 months ago

I just checked in Firefox, the bug appears for me there too.

Pertempto commented 9 months ago

Ahhh, I figured it out! I had forgotten to run npm i 🤦🏽

My currently installed tailwind didn't have the has- classes.

It displays the current value now! 👍🏽

globalbibletools / gbt