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

Adding Bio Lines + link in bio #113

Closed
wants to merge 8 commits into from

Conversation

kenjibailly
Copy link
Contributor

Proposed Changes

Up to 4 Bio lines + 1 link
New variables for #110

  • BIO_2
  • BIO_3
  • BIO_4
  • BIO_LINK

Checklist

  • Tested locally
  • Ran yarn ci to test my code
  • Did not add any unnecessary changes
  • All my changes appear after other changes in each file
  • Included a screenshot (if adding a new button)
  • 🚀

With this configuration if you don't use any of these lines, the html tags will be created but invisible to the client.
Not sure if this is a problem.

@kenjibailly
Copy link
Contributor Author

@timothystewart6 why is it failing? I don't seem to understand what exactly is failing. Tested everything locally and works fine.

@timothystewart6
Copy link
Contributor

It's failing due to invalid yaml. You can't have a blank env variable, you should just omit it. Side note, I am not sure I want to solve it this way, this limits the amount of new links you can have. I was thinking of solving it like CUSTOM_BUTTON where you have a delimiter so you can have an unlimited amount or new lines. This would then work with the exiting BIO or if you added a delimiter with values. The only downside if we will need to get creative with the delimiter since many will have , or | in their BIO. Was thinking \ or / but that might introduce other challenges with escaping :). ~ would probably work :)

@timothystewart6
Copy link
Contributor

I think ~ is the way to go. I should also update CUSTOM_BUTTON to use this too along with commas so it's backwards compatible but tell people to use ~. That way we can use ~ for everything that needs multiple values.

@timothystewart6
Copy link
Contributor

Going to close this but will think about this some more along with an implementation that is backwards compatible and consistent.

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.

2 participants