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.

Sign up for GitHub

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

Jump to bottom

TextLink component #109

Merged
merged 6 commits into from
Dec 4, 2023
Merged

TextLink component #109

merged 6 commits into from
Dec 4, 2023

Conversation

Arashayo
Copy link
Member

Zrzut ekranu 2023-07-27 133843
Zrzut ekranu 2023-07-27 133855

marcol13
marcol13 reviewed Aug 1, 2023
View reviewed changes
Copy link
Collaborator

@marcol13 marcol13 left a comment

Choose a reason for hiding this comment

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

Super robota! 😁

Bardzo fajnie to wygląda 😉 Przejrzyj moje uwagi, spróbuj zastosować tu CSS Modules oraz postaraj się dodać plik storybooka, który opisywałby ten komponent (możesz też podejrzeć jak to wygląda w już istniejących historyjkach) 😉

ui/src/components/TextLink/TextLink.jsx Outdated
@@ -0,0 +1,34 @@
import React from 'react'
import './TextLink.scss'
Copy link
Collaborator

Choose a reason for hiding this comment

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

To jest dobry sposób na import styli do pliku, ale w projekcie korzystamy z koncepcji CSS Modules (link). Zamiast pliku .scss tworzymy .module.scss i importujemy obiekt, który zawiera wszystkie style w nim zdefiniowane. Działanie w praktyce możesz zaobserwować w innych, istniejących już plikach (np. ListItem lub Button)

ui/src/components/TextLink/TextLink.jsx
<a
href={url}
target={external ? '_blank' : '_self'}
rel={external ? 'noopener noreferrer' : ''}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wow, świetnie użyłeś noopener noreferrer 😀 Fajny use-case

@marcol13 marcol13 self-requested a review August 2, 2023 19:22
PixelSculptor
PixelSculptor reviewed Aug 5, 2023
View reviewed changes
ui/src/components/TextLink/TextLink.scss Outdated
.text-link {
text-decoration: underline;
color: $alternative-a500;
padding: 10px 4px 0px 4px;
Copy link
Contributor

Choose a reason for hiding this comment

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

convert to rem or add TODO comment to refactor through mixin in the future ;)

PixelSculptor
PixelSculptor reviewed Aug 5, 2023
View reviewed changes
ui/src/components/TextLink/TextLink.scss Outdated
&:focus {
outline: none;
background-color: $primary-p100
;
Copy link
Contributor

Choose a reason for hiding this comment

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

code formatting

PixelSculptor
PixelSculptor reviewed Aug 5, 2023
View reviewed changes
ui/src/components/TextLink/TextLink.scss Outdated
}

.external-icon {
top: -0.2em;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why em here? Are u sure about this solution?

@Arashayo
Copy link
Member Author

Arashayo commented Dec 2, 2023

@PixelSculptor @marcol13
Zerknijcie proszę czy dobrze wprowadziłem zmiany i czy storybook jest odpowiedni, bo mam wrażenie, że mi się zbugował

@PixelSculptor
Copy link
Contributor

@Arashayo wygląda spoko, co do storybooka to @marcol13 bardziej ogarnia więc poczekajmy na jego feedback ;)

PixelSculptor
PixelSculptor approved these changes Dec 3, 2023
View reviewed changes
Copy link
Contributor

@PixelSculptor PixelSculptor left a comment

Choose a reason for hiding this comment

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

Jak @marcol13 da zielone światło, merguj

marcol13
marcol13 approved these changes Dec 3, 2023
View reviewed changes
Copy link
Collaborator

@marcol13 marcol13 left a comment

Choose a reason for hiding this comment

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

Jak dla mnie wygląda spoko! I w storybooku też wszystko śmiga! Approve ✅

@Arashayo Arashayo merged commit 6577e33 into frontend Dec 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PixelSculptor PixelSculptor PixelSculptor approved these changes

@marcol13 marcol13 marcol13 approved these changes

@A-Wodnicki A-Wodnicki Awaiting requested review from A-Wodnicki A-Wodnicki is a code owner

@martao02p martao02p Awaiting requested review from martao02p martao02p is a code owner

@agradecki agradecki Awaiting requested review from agradecki agradecki is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Arashayo @PixelSculptor @marcol13