Icons #30

alexkazik · 2023-09-13T17:04:53Z

As discussed in #29, here is the MR.

This comes in two parts.

I've included your first comments (from my repo).
Thanks for the response!

I've tested it some, but another test and set of eyes would be helpful.

First part

This MR with three commits:

Add icons to to the project
Use the new icons
Deprecate util::include_cdn_icons

The last is only a logical followup of the first, it can be moved into part two (or just some time later) if you prefer.

Second part

A commit with breaking changes:
alexkazik@5368bba

Since this is a breaking change, it requires a version bump to 0.6 (currently it's 0.5.18).

And that is why it will be a separate MR.

(Edit: if yew 0.21 has some breaking changes then the the version of yew-bootstrap has also to be raised to at least 0.6.)

alexkazik · 2023-09-14T06:31:49Z

After thinking about it a bit more: Should I just delete icons2 since it's less helpful than I thought?

alexkazik · 2023-09-15T10:48:04Z

I just noticed that icons v1.11.0 is out, should I update the first commit, add a new one with the update or ignore it for now?

isosphere · 2023-09-15T13:10:11Z

I just noticed that icons v1.11.0 is out, should I update the first commit, add a new one with the update or ignore it for now?

I'd just ignore it for now; there are some new icons there, but we don't need to be in a rush to support them

alexkazik · 2023-09-29T11:50:56Z

Yew 0.21 is out, and the icons require another impl, which is not available in 0.20.
Should I add it to this MR or should be merged for 0.20 first?

isosphere · 2023-09-29T15:11:10Z

Upstream Yew

Given than you introduce a breaking change, we might as well update this to Yew 0.21. Better one big expected change than two.

yew-bootstrap builds and tests fine on Yew 0.21, but you have to run cargo update first due to yewstack/yew#3366.

PR Changes

I swear I edited your README.md somewhere to clarify it for new users/quick start, but I can't find that anywhere. Does this ring a bell to you? 😅.

New Version

I think the versioning of our crate is currently an ugly mess, see #23. I might want to not use 0.6 as the next version and instead do something massively different so it makes more sense going forward.

alexkazik · 2023-09-29T16:52:48Z

I've updated this RP and added the yew update (edit: though it's incomplete, I can remove it) and also ToHtml for BI.

Yes it rings a bell, it's here: alexkazik/yew-bootstrap-icons#1 and I've included many things but not all since it's now structured a bit different.

Should I remove the example icons2 (and rename icons1 to icons) since it's as helpful as I initially thought?

Edit: I like the 0.6+y0.21-b5.3 or similar for the versioning. Interestingly enough semver.org says that behind the plus only [0-9A-Za-z-] is allowed (and thus no dot) but shows 1.0.0-beta+exp.sha.5114f85 as an example (with dot).

isosphere · 2023-10-03T23:20:15Z

I've tucked into your branch here, but it looks like I can't get your examples/icons1 to compile:

   Compiling icons1 v0.1.0 (C:\Users\Matt\Documents\Development\Rust\yew-bootstrap\examples\icons1)
error[E0277]: `yew_bootstrap::icons::BI` doesn't implement `std::fmt::Display`
 --> icons1\src\main.rs:7:44
  |
7 |         <h1>{"I"}<span style="color: red">{BI::HEART_FILL}</span>{BI::GEAR_FILL}</h1>
  |                                            --^^^^^^^^^^^^
  |                                            |
  |                                            `yew_bootstrap::icons::BI` cannot be formatted with the default formatter
  |                                            required by a bound introduced by this call
  |
  = help: the trait `std::fmt::Display` is not implemented for `yew_bootstrap::icons::BI`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
  = note: required for `yew_bootstrap::icons::BI` to implement `ToString`
  = note: required for `VNode` to implement `From<yew_bootstrap::icons::BI>`
  = note: required for `yew_bootstrap::icons::BI` to implement `Into<VNode>`
  = note: 2 redundant requirements hidden
  = note: required for `yew_bootstrap::icons::BI` to implement `Into<NodeSeq<yew_bootstrap::icons::BI, VNode>>`

error[E0277]: `yew_bootstrap::icons::BI` doesn't implement `std::fmt::Display`
 --> icons1\src\main.rs:7:67
  |
7 |         <h1>{"I"}<span style="color: red">{BI::HEART_FILL}</span>{BI::GEAR_FILL}</h1>
  |                                                                   --^^^^^^^^^^^
  |                                                                   |
  |                                                                   `yew_bootstrap::icons::BI` cannot be formatted with the default formatter
  |                                                                   required by a bound introduced by this call
  |
  = help: the trait `std::fmt::Display` is not implemented for `yew_bootstrap::icons::BI`
  = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
  = note: required for `yew_bootstrap::icons::BI` to implement `ToString`
  = note: required for `VNode` to implement `From<yew_bootstrap::icons::BI>`
  = note: required for `yew_bootstrap::icons::BI` to implement `Into<VNode>`
  = note: 2 redundant requirements hidden
  = note: required for `yew_bootstrap::icons::BI` to implement `Into<NodeSeq<yew_bootstrap::icons::BI, VNode>>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `icons1` (bin "icons1") due to 2 previous errors
2023-10-03T23:16:19.760046Z ERROR  error
error from HTML pipeline

Caused by:
    0: error from asset pipeline
    1: error during cargo build execution
    2: cargo call returned a bad status

Your icons2 also fails to compile, for a different reason (use of relative paths, I think):

~/Documents/Development/Rust/yew-bootstrap/examples/icons2> trunk serve                                                                                                     
10/03/2023 08:19:17 PMError: error taking canonical path to [watch].ignore "static/bootstrap-icons-v1.10.5" in "\\\\?\\C:\\Users\\Matt\\Documents\\Development\\Rust\\yew-bootstrap\\examples\\icons2\\Trunk.toml"

Caused by:
    The system cannot find the file specified. (os error 2)

packages/yew-bootstrap builds fine, though.

isosphere · 2023-10-03T23:27:54Z

I'm going to push the Yew 0.21 change immediately as 0.6.0. I am happy to jump right to 0.7.0 immediately after we do that with limited delay; I think this is best for our end-users using semantic versioning.

This is a breaking change!

alexkazik · 2023-10-04T07:42:44Z

I had a different problem (couldn't build yew-0.21) first (all examples).
Then I did a cargo update.
And afterwards I had no problem building all examples (including mine).

Since the Cargo.lock file is not in the repo I can't reproduce your problem.
It is recommended to add the lock file for binaries (the examples) to repos.

And jonhoo also adds then to his libraries so that the CI jobs can work with it. Which is what I have adopted.

I've pushed the changes on the new main.
Also I've removed the icons2 example (I'll re-add it if you want, but I think it's not a good idea).

isosphere · 2023-10-04T11:59:19Z

Please don't use force push, it messes up source trees external to your own. Let the git history be messy!

#8 (comment)

alexkazik · 2023-10-05T05:42:21Z

I'm sorry - I'm used to fix it until it's right.

isosphere · 2023-10-19T17:50:50Z

Hey @alexkazik; I've made some edits. Please review my changes and correct me if necessary. Otherwise, I'd be happy to proceed with merging and releasing as 0.7.0.

alexkazik · 2023-10-20T06:25:03Z

Looks good. Though now I'm wondering if the LICENSE should be added to BIFiles as well (and thus also be in the dist of the application)?
If yes, I can put a commit with that together.

isosphere · 2023-10-20T12:41:11Z

Looks good. Though now I'm wondering if the LICENSE should be added to BIFiles as well (and thus also be in the dist of the application)?
If yes, I can put a commit with that together.

I think you're right; binary distributions must have the license as well.

We might have to include several licenses one day, I wonder how we can do that without losing clarity about why each license is included. Folders, I guess?

alexkazik · 2023-10-21T07:31:43Z

I've added a few things to a new branch, and are happy to move them here if you want:
/~https://github.com/alexkazik/yew-bootstrap/commits/icons-license

add license to BIFiles (and copy)
fix clippy warnings
remove BI::empty/default

I've added BI::empty as a shortcut for myself. I don't know if it's a good idea to support an icon and no icon in one.
When removed you have to use Option which looks more matching but probably makes it harder to work with: you have to unwrap_or yourself.
Alternative: feature flag to enable empty
Alternative: support a second type, one with one without empty (BI+BINonEmpty or BI+BIE(mpty))
As removing empty is a breaking change I wanted to bring it up now.

alexkazik · 2023-10-29T15:56:58Z

I've added a few more commits to the icons-license branch:

Fix typo
TESTING! use upcoming yew !TESTING
Replace ToHtml implementation with IntoPropValue

The first is just to remove an unbalanced backtick.

Then yew is removing the ToHtml trait (already in master).
The one commit uses the current version and the other fixes the icons implementation for it.

Please tell me which I should add to this PR (or you can pick them yourself).

isosphere · 2023-11-11T16:24:57Z

I've added a few more commits to the icons-license branch:
* Fix typo

* TESTING! use upcoming yew !TESTING

* Replace ToHtml implementation with IntoPropValue
The first is just to remove an unbalanced backtick.

Then yew is removing the ToHtml trait (already in master). The one commit uses the current version and the other fixes the icons implementation for it.

Please tell me which I should add to this PR (or you can pick them yourself).

Please do not add a change in the yew dependency to this PR; one PR per "thing" please. The license change, clippy warnings, typo fixes, and the removal of BI::empty seem worthy.

alexkazik · 2023-11-11T18:53:15Z

I've added the mentioned commits to this RP.

The upcoming yew changes are now on icons-upcoming-yew, for later.

isosphere · 2023-11-11T22:23:04Z

I've added the mentioned commits to this RP.

The latest commits I see attached to this PR are on Oct 29, 2023; might have forgotten to push?

alexkazik · 2023-11-11T22:35:36Z

The latest commits I see attached to this PR are on Oct 29, 2023;

Thats all right - they are that old ;-)

isosphere · 2023-11-12T00:50:27Z

Thank you for your contribution! This has been published as version 0.7.0.

isosphere mentioned this pull request Sep 29, 2023

Upgrade to Yew 0.21 #31

Closed

isosphere added this to the 2023Q4R1 milestone Sep 29, 2023

isosphere linked an issue Sep 29, 2023 that may be closed by this pull request

Icons #29

Closed

alexkazik force-pushed the icons branch from 5ee5686 to 5921eac Compare September 29, 2023 16:45

alexkazik force-pushed the icons branch from 5921eac to f643beb Compare September 29, 2023 16:54

isosphere modified the milestones: 0.6.0, 0.7.0 Oct 3, 2023

alexkazik force-pushed the icons branch from f643beb to ace5a5d Compare October 4, 2023 07:25

alexkazik added 4 commits October 4, 2023 09:32

Add icons to to the project

5b1a41d

Use the new icons

6b9f70f

Deprecate util::include_cdn_icons

9766892

Use BI in NavBar

bdb783b

This is a breaking change!

alexkazik force-pushed the icons branch from ace5a5d to bdb783b Compare October 4, 2023 07:33

Include LICENSE from bootstrap-icons, as required by their LICENSE.

3360ef0

isosphere self-requested a review October 19, 2023 17:28

isosphere added 3 commits October 19, 2023 14:35

README.md edits

4a9a44a

I don't want to remember to update this comment for every yew update!

af91f95

Doc edits

ed5a75b

alexkazik added 4 commits October 29, 2023 16:53

Fix typo

0df505e

Add License to BIFiles

e6fea45

Fix clippy warnings

af2ba8d

Remove BI::empty/default

f147396

isosphere added 3 commits November 11, 2023 20:42

Remove BI::Empty from doc

a2bd6f0

Merge branch 'main' into pr/alexkazik/30

4236c67

Version bump + lock update

b810894

isosphere merged commit bb19075 into isosphere:main Nov 12, 2023
1 check passed

alexkazik deleted the icons branch November 12, 2023 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Icons #30

Icons #30

alexkazik commented Sep 13, 2023 •

edited

Loading

alexkazik commented Sep 14, 2023

alexkazik commented Sep 15, 2023

isosphere commented Sep 15, 2023

alexkazik commented Sep 29, 2023

isosphere commented Sep 29, 2023 •

edited

Loading

alexkazik commented Sep 29, 2023 •

edited

Loading

isosphere commented Oct 3, 2023 •

edited

Loading

isosphere commented Oct 3, 2023

alexkazik commented Oct 4, 2023

isosphere commented Oct 4, 2023 •

edited

Loading

alexkazik commented Oct 5, 2023

isosphere commented Oct 19, 2023

alexkazik commented Oct 20, 2023

isosphere commented Oct 20, 2023

alexkazik commented Oct 21, 2023

alexkazik commented Oct 29, 2023

isosphere commented Nov 11, 2023

alexkazik commented Nov 11, 2023

isosphere commented Nov 11, 2023

alexkazik commented Nov 11, 2023

isosphere commented Nov 12, 2023

Icons #30

Icons #30

Conversation

alexkazik commented Sep 13, 2023 • edited Loading

First part

Second part

alexkazik commented Sep 14, 2023

alexkazik commented Sep 15, 2023

isosphere commented Sep 15, 2023

alexkazik commented Sep 29, 2023

isosphere commented Sep 29, 2023 • edited Loading

Upstream Yew

PR Changes

New Version

alexkazik commented Sep 29, 2023 • edited Loading

isosphere commented Oct 3, 2023 • edited Loading

isosphere commented Oct 3, 2023

alexkazik commented Oct 4, 2023

isosphere commented Oct 4, 2023 • edited Loading

alexkazik commented Oct 5, 2023

isosphere commented Oct 19, 2023

alexkazik commented Oct 20, 2023

isosphere commented Oct 20, 2023

alexkazik commented Oct 21, 2023

alexkazik commented Oct 29, 2023

isosphere commented Nov 11, 2023

alexkazik commented Nov 11, 2023

isosphere commented Nov 11, 2023

alexkazik commented Nov 11, 2023

isosphere commented Nov 12, 2023

alexkazik commented Sep 13, 2023 •

edited

Loading

isosphere commented Sep 29, 2023 •

edited

Loading

alexkazik commented Sep 29, 2023 •

edited

Loading

isosphere commented Oct 3, 2023 •

edited

Loading

isosphere commented Oct 4, 2023 •

edited

Loading