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

Improve button spacing in specupload #1708

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Conversation

themerekat
Copy link
Collaborator

@themerekat themerekat commented Sep 10, 2024

Pull Request Checklist:

From this:
image

To this:
image

Fixes: #1707

Pre-Approval

  • There is a description section in the pull request that details what the proposed changes do. It can be very brief if need be, but it ought to exist.
  • Features and backlog bugs should be merged into the Development branch, NOT master
  • All new text is preferably internationalized (i.e., no end-user-visible text is hard-coded on the PHP pages), and the spreadsheet tracking internationalizations has been updated either with a new row or with checkmarks to existing rows.
  • There are no linter errors
  • Symbiota coding standards have been followed
  • Comment which GitHub issue(s), if any does this PR address

Post-Approval

  • It is the code author's responsibility to merge their own pull request after it has been approved
  • If this PR represents a merge into the Development branch, remember to use the squash & merge option
  • If this PR represents a merge from the Development branch into the master branch, remember to use the merge option
  • If this PR represents a hotfix into the master branch, a subsequent PR from master into Development should be made merge option (i.e., no squash).
  • If the dev team has agreed that this PR represents the last PR going into the Development branch before a tagged release (i.e., before an imminent merge into the master branch), make sure to notify the team and lock the Development branch to prevent accidental merges while QA takes place. Follow the release protocol here.
  • Don't forget to delete your feature branch upon merge. Ignore this step as required.

Thanks for contributing and keeping it clean!

Copy link
Member

@GregoryPost GregoryPost 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 are trying to get away from inline styling. I suggest adding a class to those buttons and then styling the class in main.css

@themerekat
Copy link
Collaborator Author

@GregoryPost , the styling is putting them in divs, not in-line with the buttons. Are you saying that we should just add blanket padding to all buttons?

@GregoryPost
Copy link
Member

Looks like there already is a class that could serve this purpose: .bottom-breathing-room-rel

Example:
<button class="bottom-breathing-room-rel" type="submit" name="action" value="Automap Fields" >

@GregoryPost
Copy link
Member

@GregoryPost , the styling is putting them in divs, not in-line with the buttons. Are you saying that we should just add blanket padding to all buttons?

No, just updating those buttons with an appropriate css class selector. If you do this you should not need the new divs with inline styling.

@NikitaSalikov
Copy link
Collaborator

@themerekat, You can also use a smaller gap class: "bottom-breathing-room-rel-sm" which might look more aesthetic.

@GregoryPost GregoryPost changed the base branch from Development to hotfix September 11, 2024 00:08
@themerekat themerekat closed this Sep 11, 2024
@themerekat themerekat deleted the specupload-button-spacing branch September 11, 2024 20:49
@themerekat themerekat restored the specupload-button-spacing branch September 11, 2024 20:50
@themerekat
Copy link
Collaborator Author

Still not super sure I like it, because now the Start Upload button is a little askew of these buttons, but it works.

@themerekat themerekat merged commit 29bfa66 into hotfix Sep 12, 2024
@themerekat themerekat deleted the specupload-button-spacing branch September 12, 2024 13:06
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.

3 participants