-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
rounded buttons, added hover effects, also hide and show toggle icons #259
Conversation
rounded buttons, added hover effects, also hide and show toggle icons
@kekubhai is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
@jamesread @nevo-david If you are up to review this, then my guess is it would be great to merge such changes as fast as possible not to have potential merge conflicts at later stages. Looking at the provided screenshots, this seems to be fixing only: #257 |
@kekubhai Thanks very much for this PR, this LGTM. The CI run failed (https://github.com/gitroomhq/postiz-app/actions/runs/11058616188/job/30957330622?pr=259), so I've tried a branch update and resubmitted to CI. |
Thanks sir. It was an honor to contribute here. Will check for more issues here. Also when can I expect a hacktoberfest accepted label if you don't mind. |
Well the CI run hasn't completed successfully, so the issue isn't resolved yet. Please also answer my code review comment. |
I don't know what is causing this error. Do tell me if there's anything from my side . Also could you please tell me which review comment you are referring to, I couldn't see any. |
Your lockfiles are different. This is an npm issue.
|
I see. I don't know much about this . Is there something from my side which I can do ? Please let me know. |
Well it seems that your commits contain a change in the dependencies for the project. Specifically some dependencies are removed from the lockfile. And @jamesread this is the most likely cause for the failing CI. |
oh yes this maybe the issue. I had to install lucide-react to import the icon . Is there Any way this can be sorted ? |
hey @kekubhai check from postiz GitHub and cross check all the dependencies and lock file |
Hey, I'm really sorry, but this PR changes too many things at once, and it's now got conflicts and package-lock.json issues. I'm going to close this PR (you will still keep your code changes on your branch), and could I recommend you either re-write the changes as clean commits against new feature branches, or do a Git Cherry Pick of the changes into new feature branches, and raise PRs separately for those. I've also just updated the pull request template to ask that people also raise one issue per PR as well. I did merge #274 which fixes the rounded corners in a quick and easy way, so apologies that PR "beat" this part of your PR. |
What kind of change does this PR introduce?
eg: Bug fix, feature, docs update, ...
This pull request solves the button ussue and add toggle icon with password with some eye-catching hover effects for user experience
Why was this change needed?
Please link to related issues when possible.
this resolve the issue :#258 and issue : #257
Other information:
eg: Did you discuss this change with anybody before working on it (not required, but can be a good idea for bigger changes). Any plans for the future, etc?
yes i discussed the changes in discord and the mods really agreed to it.
here's a demo :