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

Fixed issue 211 #390

Merged
merged 9 commits into from
Apr 15, 2022
Merged

Fixed issue 211 #390

merged 9 commits into from
Apr 15, 2022

Conversation

alicendeh
Copy link
Contributor

@alicendeh alicendeh commented Apr 8, 2022

Summary

Realigned buttons in project page and reduced vertical spacing between clab,bookmark,view button row and photos div

Changes

  • Realigned buttons in Project Page
  • Reduced vertical spacing between clab,bookmark,view button row and photos div

Screenshots

Screenshot from 2022-04-08 05-28-29

fixed failed to load api specification error (unstructuredstudio#363)

fixed failed to load api specification error

added style to fix text wrap (unstructuredstudio#373)

Add hot reloading to media container (unstructuredstudio#382)

Add new search features (unstructuredstudio#362)

* Finish creators and projects search migration

* Finish creators and projects search migration

* Add tag search

* Add search for projects with tags

* Fix search bar on mobile

* use 0 rather than 0 with units in css

* Remove print

* Default search type to projects

* Disable scroll lock

Revert package-lock (unstructuredstudio#383)

improveConsistency: fixed title text small, extra vertical space ... (unstructuredstudio#369)

* improveConsistency: fixed title text small, extra vertical space and inconsitent padding of buttons

fix text overflow issue in signup page phone number field  (unstructuredstudio#378)

removed spacing between icons
@alicendeh
Copy link
Contributor Author

Here @NdibeRaymond. I closed the other PR and opened this one. Please have a look

@tuxology
Copy link
Member

tuxology commented Apr 8, 2022

Thanks @alicendeh . The Clap-bookmark-views "button" does not look ok still since its all squashed together on the left while there is empty space on right. I have some ideas to really make this way better. It would go a bit out of scope of #211 's requirements.

So for now, essentially we could just right align the complete button (containing all 3 elements) and reduce width so that it looks more balanced. Also introduce a bit of padding between text and the icon as well. This will do for now and we can improve this in another iteration.

@NdibeRaymond
Copy link
Collaborator

@alicendeh when you upload the patch for @tuxology 's review we will merge this.

@alicendeh
Copy link
Contributor Author

Thanks @alicendeh . The Clap-bookmark-views "button" does not look ok still since its all squashed together on the left while there is empty space on right. I have some ideas to really make this way better. It would go a bit out of scope of #211 's requirements.

So for now, essentially we could just right align the complete button (containing all 3 elements) and reduce width so that it looks more balanced. Also introduce a bit of padding between text and the icon as well. This will do for now and we can improve this in another iteration.

Got it. Working on that.

@alicendeh
Copy link
Contributor Author

@alicendeh when you upload the patch for @tuxology 's review we will merge this.

okay, @NdibeRaymond. I was working on the mobile section so I couldn't modify this the other day. But am on it now.

@alicendeh
Copy link
Contributor Author

Hello @tuxology and @NdibeRaymond. I did the necessary corrections requested.

  1. Gave the same padding between the clap-bookmark and view icon
  2. Added spacing between the text and the icon
  3. Aligned the items to the right

OUTPUT

Screenshot from 2022-04-11 08-25-36

@tuxology
Copy link
Member

This looks better. We are almost there! Can we remove full width of the "clap-bookmark-views" element and just have it enough to cover the 3 "buttons" (and the whole button remains aligned right as well)?

@alicendeh
Copy link
Contributor Author

This looks better. We are almost there! Can we remove full width of the "clap-bookmark-views" element and just have it enough to cover the 3 "buttons" (and the whole button remains aligned right as well)?

Please @tuxology am a little lostZ
What do you mean by full width please?

@tuxology
Copy link
Member

tuxology commented Apr 12, 2022

@alicendeh We do something like this for now

image

This design will eventually change in a few months as we optimize the position of these elements, but for now, this will help!

@alicendeh
Copy link
Contributor Author

Alright, I see now, Thanks.
I will provide these change in a while

@alicendeh
Copy link
Contributor Author

alicendeh commented Apr 13, 2022

Hello @tuxology I have done the change you requested

OUTPUT
Screenshot from 2022-04-13 07-23-20

@alicendeh
Copy link
Contributor Author

Hello @tuxology please you havent responded to this yet

Copy link
Member

@tuxology tuxology left a comment

Choose a reason for hiding this comment

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

LGTM 🏳️‍🌈 Lets checkout package-lock.json from master branch and then merge it

@alicendeh
Copy link
Contributor Author

Hello @tuxology the content of the package-lock.json is same as whats currently on the repo

@tuxology
Copy link
Member

tuxology commented Apr 15, 2022

@alicendeh Looks like there is a difference (https://github.com/unstructuredstudio/zubhub/pull/390/files#diff-1e460fd2de8f7f6c83ff9c821068937ff5f1be7769d2ec1679a1280f19d8263a). Did you update your master branch locally before diffing? The caniuse-lite package seems to have been updated

@alicendeh
Copy link
Contributor Author

@tuxology please have a look now

Copy link
Member

@tuxology tuxology left a comment

Choose a reason for hiding this comment

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

LGTM 🏳️‍🌈

@ajain1921
Copy link
Contributor

@alicendeh @tuxology It seems like this PR adds the ability to select multiple categories on the Create Project Form. I believe it's leading to Issue #424. Is this a feature we want? For now, I'm going to remove the multiple category addition to fix the issue and maybe discuss if this is something we want in the future.

@tuxology
Copy link
Member

@ajain1921 I think yes, you are right. I merged too soon. This has some stale code from the other category issue (possibly should have been included in #395) that we decided not to merge. Thanks for removing he multiple category code that got included in this PR.

@alicendeh Please take a note for next PRs that they should not have stale code from other issues. A general approach is to always create separate feature branched from the main master branch and keep your feature branch up to date. Unless the features are interlinked (which is not the case here) never branch from your own active feature branch. This can cause issues like the one we see here

@alicendeh alicendeh deleted the isssue211 branch April 23, 2022 01:38
@alicendeh
Copy link
Contributor Author

Noted @tuxology thqnks. This won't happen again

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.

4 participants