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

Update Link Titling by client #27

Merged

Conversation

raphaelbernhart
Copy link
Contributor

Here is a small (mostly UX) Feature which I think which improves the User Experience by giving more control of a link to the user.

The Problem: Sometimes I create a Link but I am not happy with the default titling of a link (the website title). F.e. I want to save links to job postings but don't want the website title of the recruitment platform as the link title.

With this feature you can add a custom title to the link (optional - then the default way of titling will be used).
Also I changed the UI of the Update-Title component input element for UX reasons. First I haven't understand that I can change the title because of the similarity of the title input element to a normal headline.

Changelog

  • update styling of Update-Link title input tag
  • add Link Titling by client on creation

@BetaHuhn BetaHuhn changed the base branch from master to develop August 29, 2021 15:52
@BetaHuhn
Copy link
Member

BetaHuhn commented Aug 29, 2021

Hey, thanks for taking the time to create this PR!

I like the first change being able to add a title when creating a link, it can definitely save some time and doesn't disturb otherwise.

I am going to reject the second change for now. I think it is unnecessary to constantly display an input for the link title in the details view and I don't like how it looks. I get your point though and will think about ways to indicate that the title can be changed.

If you remove the styling change (commit 46d8c30) I will merge this PR.

And again for the future: It's best to create separate PRs for different changes, makes it easier to review :)

@raphaelbernhart
Copy link
Contributor Author

Hello, yeah I understand what u mean I also thought some time about the second change. I just didn't found any other way to solve this problem.

I made those two changes in one PR because those are two similiar problems.

@BetaHuhn
Copy link
Member

I made those two changes in one PR because those are two similiar problems.

No worries, it's just that as you see the one change could be merged directly while the other needs some additional work. Reducing PRs to only the most relevant change/a single commit makes easier and faster to review as I can go through one change at a time.

No problem though, just remove the one commit or create a new PR and I will include this in the next release (should be out tomorrow) 😄

@BetaHuhn BetaHuhn merged commit 3c1884a into WebCrateApp:develop Aug 30, 2021
@BetaHuhn
Copy link
Member

Thanks again!

@WebCrateBot
Copy link
Collaborator

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants