Skip to content

Adds Miso.Style#920

Merged
dmjio merged 40 commits into
masterfrom
miso-style
May 9, 2025
Merged

Adds Miso.Style#920
dmjio merged 40 commits into
masterfrom
miso-style

Conversation

@dmjio
Copy link
Copy Markdown
Owner

@dmjio dmjio commented May 6, 2025

  • Add additional combinators / strong types
  • Update examples to use new style_ combinator
  • Adds ISO colors via Miso.Style.Color
  • Preserve existing style_ HTML combinator (this is why Miso.Style must be used qualified)

Addresses #746

@jhrcek

dmjio and others added 3 commits May 6, 2025 15:22
- [ ] Add additional combinators / strong types
- [ ] Update examples to use new style_ combinator
- [x] Add Miso.Style.Color
@dmjio
Copy link
Copy Markdown
Owner Author

dmjio commented May 6, 2025

@functora

@dmjio dmjio requested a review from Copilot May 6, 2025 23:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 2 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (13)
  • examples/fetch/Main.hs: Language not supported
  • examples/mario/Main.hs: Language not supported
  • examples/svg/Main.hs: Language not supported
  • examples/websocket/Main.hs: Language not supported
  • haskell-miso.org/shared/Common.hs: Language not supported
  • miso.cabal: Language not supported
  • src/Miso.hs: Language not supported
  • src/Miso/Html/Element.hs: Language not supported
  • src/Miso/Html/Property.hs: Language not supported
  • src/Miso/Html/Types.hs: Language not supported
  • src/Miso/Property.hs: Language not supported
  • src/Miso/Svg/Attribute.hs: Language not supported
  • src/Miso/Svg/Element.hs: Language not supported

Comment thread src/Miso/Style.hs
Comment thread src/Miso/Style.hs Outdated
@dmjio dmjio force-pushed the miso-style branch 4 times, most recently from 11bef52 to 2db7d67 Compare May 7, 2025 16:16
Comment thread src/Miso/Property.hs
intProp = prop
-----------------------------------------------------------------------------
-- | Set field to `Integer` value
integerProp :: MisoString -> Int -> Attribute action
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
integerProp :: MisoString -> Int -> Attribute action
integerProp :: MisoString -> Integer -> Attribute action

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Let's keep it Int, if you want to rename it then do intProp

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

but honestly, probably best to just leave it for now, since its a big breaking change

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But then the function name and haddock above it are lying.
What's the purpose of having both intProp and integerProp with identical type signature?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I believe BigInt wasn’t supported at the time miso was released. The Integer implementation was much slower, I’ll need to look into this more.

Comment thread src/Miso/Property.hs
stringProp = prop
-----------------------------------------------------------------------------
-- | Set field to `Text` value
textProp :: MisoString -> MisoString -> Attribute action
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
textProp :: MisoString -> MisoString -> Attribute action
textProp :: MisoString -> Text -> Attribute action

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This has to stay MisoString

Copy link
Copy Markdown
Contributor

@jhrcek jhrcek May 7, 2025

Choose a reason for hiding this comment

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

in that case just update the Haddock above this not to mention Text 🙂

Copy link
Copy Markdown
Owner Author

@dmjio dmjio May 7, 2025

Choose a reason for hiding this comment

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

MisoString becomes Text on GHC and stays JSString on WASM / JS

Comment thread src/Miso/Style.hs Outdated
Comment thread src/Miso/Style.hs
-- | 'StyleSheet'
--
-- Type for a CSS style on native
--
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's Map from MisoString to Styles, which itself is Map from MisoString to Miso String.
Maybe add a sentence about what the keys / values at each level are? The same for sheet_ below.

Comment thread src/Miso/Style.hs
Comment on lines +270 to +272
-- | 'renderStyleSheet'
--
-- Renders a 'StyleSheet' to a 'MisoString'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment belongs to the function below?

Comment thread src/Miso/Style.hs
, (=:)
-- *** Render
, renderStyleSheet
-- *** Combinators
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just curious: what is your source for this list of css properties you're creating combinators for?

dmjio and others added 3 commits May 8, 2025 15:05
Co-authored-by: Jan Hrcek <2716069+jhrcek@users.noreply.github.com>
@dmjio dmjio merged commit 147e7c7 into master May 9, 2025
3 checks passed
@dmjio dmjio deleted the miso-style branch May 9, 2025 00:23
@dmjio
Copy link
Copy Markdown
Owner Author

dmjio commented May 9, 2025

We'll come back to this, we might take a full lexer / parser approach to CSS. Have to move back onto docs and native, we'll keep fleshing it out piecemeal on-demand.

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