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

Blockquote component #461

Closed
Robbert opened this issue Jul 31, 2024 · 5 comments · Fixed by #537
Closed

Blockquote component #461

Robbert opened this issue Jul 31, 2024 · 5 comments · Fixed by #537
Assignees

Comments

@Robbert
Copy link
Member

Robbert commented Jul 31, 2024

Figma
Blockquote in NLDS - Rijksoverheid - Bibliotheek

@Rozerinay
Copy link
Contributor

PR: #506

@Rerbun Rerbun self-assigned this Aug 14, 2024
@Rerbun Rerbun moved this from Todo to In Progress in Community Sprint - Rijkshuisstijl componenten Aug 14, 2024
@Rerbun Rerbun closed this as completed in 08f3eb0 Aug 19, 2024
MeesD94 pushed a commit that referenced this issue Aug 22, 2024
closes #461

---------

Co-authored-by: Ruben Smit <[email protected]>
@AlineNap AlineNap assigned AlineNap and unassigned Rerbun Sep 3, 2024
@AlineNap
Copy link
Contributor

Nieuwe feature request voor de blockqoute:

We willen graag dat het mogelijk is om de nieuwe variant van de blockqoute te maken en de oude van de Fijkshuisstijl. Hiervoor moeten de volgende tokens worden toegevoegd aan het component.

  • utrecht/rhc/new.blockqoute.border-radius
  • blockquote.border.color
  • blockqoute.border-width

Daarnaast de mogelijkheid om wel of geen border aan bepaalde kanten te tonen, graag op basis van tokens zodat er van thema (oud/nieuw) geswitcht kan worden. Ik heb de verschillende varianten toegevoegd aan figma.

@Robbert heeft wellicht bovenstaande tokens al aan het utrecht component toegevoegd. Vraag of kijk of dat klopt, ze mogen sowieso op de utrecht code toegevoegd worden. De mogelijkheid om een border aan of uit te zetten en aan 1 kant een border-radius te hebben is wel Rijkshuisstijl specifiek.

En dan heb ik een vraag, hebben jullie de token utrecht.blockquote.row-gap toegevoegd aan het component? Of zit deze in het oorspronkelijke component van utrecht?

Image

@AlineNap
Copy link
Contributor

Een PR van mij met deze extra border tokens met een utrecht prefix is al gemerged. Gaat om deze: #634

AdhamAH pushed a commit that referenced this issue Sep 26, 2024
@AlineNap
Copy link
Contributor

Feedback design review wordt verwerkt in:

@AlineNap
Copy link
Contributor

AlineNap commented Nov 15, 2024

Design review 15 November

  • 1 appereance van de block-quote is genoeg, die kan de betreffende Rijksorganisatie dan vervolgens stijlen naar wens doormiddel van de tokens.
  • Tokens die wij zelf toevoegen graag de pre-fix rhc geven voor het onderscheid. Ik ga er vanuit dat we tokens hebben toegevoegd, als dat niet zo is let me know :).

Vragen

  • Ik zie dat er pakketjes zijn gemaakt om de verschillende varianten te creëren. Ik ben benieuwd hoe dit werkt, zijn dit overschrijvingen?
  • Zit het ticket van consistente oplossing keep hierin? (Ga zo dit ticket bekijken).

Opmerkingen

  • Als je tokens toevoegt is het fijn als je de type invult in de json. Dan komt het voor ons gelijk in de juiste categorie in token studio/ Figma. --> Al is het voor deze pakketjes wellicht juist wel goed dat ze apart staan bedenk ik me nu. Wacht even tot ik begrijp waarom deze pakketjes er zijn a la vraag 1 ;).
},
        "border-start-start-radius": {
          "value": "3rem",
          "type": "deze graag invullen, in dit geval: borderRadius"
        },

Image

@AlineNap AlineNap changed the title Blockquote component Blockquote component (zie comment design review) Nov 15, 2024
@AdhamAH AdhamAH reopened this Nov 15, 2024
@JoeriRoijenga JoeriRoijenga self-assigned this Nov 27, 2024
@JoeriRoijenga JoeriRoijenga removed their assignment Nov 28, 2024
@Aref-Akminasi Aref-Akminasi self-assigned this Dec 4, 2024
@Rerbun Rerbun changed the title Blockquote component (zie comment design review) Blockquote component Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

7 participants