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

Enhancement/update tailwind #544

Merged
merged 22 commits into from
May 28, 2021
Merged

Enhancement/update tailwind #544

merged 22 commits into from
May 28, 2021

Conversation

henriquecbuss
Copy link
Member

What issue does this PR close

Closes #533

Changes Proposed ( a list of new changes introduced by this PR)

How to test ( a list of instructions on how to test this PR)

Browse around the app, and check if anything looks different than it used to. Everything should still look pretty much the same. The biggest change is in form elements (text input, checkboxes, radio buttons, etc.). You can look for them in these pages:

  • /community/objectives/:objective_id/action/:action_id/edit (text fields, switches, checkboxes, radio buttons)
  • /profile/add-contact (text fields, flag selects)

@henriquecbuss henriquecbuss requested a review from lucca65 May 27, 2021 18:04
Copy link
Member

@lucca65 lucca65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is the idea behind this PR, maybe not, but I found a few weird things on inputs:

  • On the transfer page, when I focus on the first input (transfer to) I get a pretty highlight, but not on the other two fields.
  • In fact the only time we got that purple highlight is in that page, afaik
  • After creating an objetive, I need to do a full reload to see my newly created objetive (maybe this is related to the refactor we did to use only one community?

lucca65
lucca65 previously approved these changes May 27, 2021
Copy link
Member

@lucca65 lucca65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outside of those points, that can be addressed in a separate issue, if you prefer, I found nothing else, animations on some elements look smoother (or maybe I was paying more attention to them? 🤔 )

@henriquecbuss
Copy link
Member Author

henriquecbuss commented May 27, 2021

On the transfer page, when I focus on the first input (transfer to) I get a pretty highlight, but not on the other two fields.
In fact the only time we got that purple highlight is in that page, afaik

The main purpose of this PR was to just update tailwind. The plugin we were using for forms before isn't supported on v2.0, instead we can use their new forms plugin, which serves more as a reset to form elements instead of actually styling them in some way (which is what the old plugin used to do), and it doesn't use classes for that - it uses the input type - which means all the classes from the old plugin now don't exist. I've recreated the classes we were using from that old plugin (hence a bunch of changes in main.css) so the inputs that were using them still look the same.

The problem is, not every input uses these classes (in reality, every input should probably be from View.Form, so we have consistent styling overall). Maybe we should have another issue to handle that. I think we should have this issue as isolated as possible, so we don't break anything (I had to update some other libraries related to CSS as well, for them to be compatible with tailwind 2.0).

After creating an objetive, I need to do a full reload to see my newly created objetive (maybe this is related to the refactor we did to use only one community?

That's because of the delay to register things on the blockchain. The difference is that we don't fetch the community again on every page, so even if we go to some other page and then come back, the objective is still not there. I think the best way to solve this would be to use that GraphQL subscription 😬. For now we could reload the community on the pages that need it though, if you think that's better

@lucca65
Copy link
Member

lucca65 commented May 27, 2021

well, can't we just adjust the css for that sole poor input?

about the objetive.. yeah you are right, but before we were reloading the community more often so it wouldn't appear as frequently.. maybe we can trigger an update when the user open the settings? would it take a lot of effort?

@henriquecbuss
Copy link
Member Author

well, can't we just adjust the css for that sole poor input?

Sure, I'll do it!

would it take a lot of effort?

Not at all, we even have a External msg on loggedIn just for that 😄

@henriquecbuss
Copy link
Member Author

@lucca65 there are still some places that don't use the default input, but I think we can do that in another issue. Or do you think we should look for all of them in this issue?

Copy link
Member

@lucca65 lucca65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we can move forward, we can tinker with those stuff later. Our app looks much better and we will have plenty of opportunities to do those fixes

@henriquecbuss henriquecbuss requested a review from heltonlr May 28, 2021 01:37
@henriquecbuss
Copy link
Member Author

@heltonlr can you test this one on the netlify deploy generated by Github?

@lucca65 lucca65 merged commit cea4b49 into master May 28, 2021
@lucca65 lucca65 deleted the enhancement/update-tailwind branch May 28, 2021 17:41
@heltonlr
Copy link

Hey @NeoVier , what exactly do you want to test?

@henriquecbuss
Copy link
Member Author

Oh, sorry bro, no need to test this anymore, we've already merged it. Take some time to rest and relax!

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.

Update TailwindCSS
3 participants